Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix race condition in status job status checking #56

Merged
merged 4 commits into from
Jan 23, 2024

Conversation

dmgaldi
Copy link
Contributor

@dmgaldi dmgaldi commented Jan 9, 2024

Description

  • I ended up making the status lazily initialized for S3 and QueueDB and added synchronization while loading the statuses
  • I also allow expiration of remotely owned jobs which should be fine once the status synchronization is fixed.

PR Checklist

  • Updated relevant source docs
  • Updated readme / docs
  • Updated dependencies

@dmgaldi dmgaldi self-assigned this Jan 9, 2024
@dmgaldi dmgaldi requested a review from Foxcapades January 9, 2024 17:25
import java.util.concurrent.locks.ReentrantLock
import kotlin.concurrent.withLock

internal object JobManager {
private val lock = ReentrantLock()
private val Log = LoggerFactory.getLogger(javaClass)
private val LocksByJobID = ConcurrentHashMap<String, ReentrantLock>()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where are entries in this map removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to use a library based on Guava's cache that will auto-expire entries. It seems to be pretty well-maintained and is relatively light-weight.

@dmgaldi dmgaldi requested a review from Foxcapades January 11, 2024 19:40
@dmgaldi dmgaldi merged commit 0527646 into main Jan 23, 2024
1 check passed
@dmgaldi dmgaldi deleted the status-synchronization branch January 23, 2024 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants