fix(job): key singleton locks by job identity#2042
Open
adityathebe wants to merge 1 commit into
Open
Conversation
Benchstat (RLS)Base: 📊 6 minor regression(s) (all within 5% threshold)
✅ 4 improvement(s)
Full benchstat output |
Benchstat (Other)Base: 📊 1 minor regression(s) (all within 5% threshold)
✅ 1 improvement(s)
Full benchstat output |
Contributor
WalkthroughThe Job struct's single ID field is replaced with a computed ID() method and an Aliases slice. A shared keyed singleton lock registry replaces per-job mutex locking. Property lookup and naming become alias-aware. The cron API exposes JobID and Aliases. New tests validate identity, property lookup, and singleton concurrency behavior. ChangesJob identity, singleton locking, and property lookup refactor
Sequence Diagram(s)sequenceDiagram
participant JobA as Job (ResourceID X)
participant JobB as Job (ResourceID X)
participant Registry as singletonLocks
JobA->>Registry: TryLock(ID())
Registry-->>JobA: locked=true
JobA->>JobA: execute (running)
JobB->>Registry: TryLock(ID())
Registry-->>JobB: locked=false (already running)
JobB->>JobB: mark StatusSkipped
JobA->>Registry: unlock()
JobA->>JobA: mark StatusSuccess
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
adityathebe
force-pushed
the
codex/keyed-singleton-locks
branch
from
049ac24 to
54edf9d
Compare
Gavel resultsGavel exited with code . |
adityathebe
force-pushed
the
codex/keyed-singleton-locks
branch
2 times, most recently
from
3c6b0b6 to
aea7ce6
Compare
Manual config-db scraper runs create a fresh duty job while scheduled runs use another instance. The old singleton mutex lived on the Job struct, so jobs with the same logical scraper identity could overlap despite Singleton being enabled. Use Job.ID() as the single computed in-process singleton identity, derived as Name::ResourceType/ResourceID when a resource is present. Replace the old stored alias field with Aliases, which is only used for property lookup and display names. Keep GetProperty and GetPropertyInt backward compatible: jobs.<name>.<property> continues to win over alias-specific values, and these accessors do not inherit the global jobs.<property> fallback. Addresses flanksource/config-db#2300
adityathebe
force-pushed
the
codex/keyed-singleton-locks
branch
from
aea7ce6 to
6fcbb98
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@job/job.go`:
- Around line 435-438: The deferred semaphore release log in the job semaphore
handling uses Infof with a prebuilt message string, which can misinterpret
percent signs in j.ID() as format tokens. Update the deferred closure around the
semaphore Release in job/job.go to log the message as data instead of a format
string by using the logger with a literal format and the assembled msg variable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35a4e93c-d801-4f21-ba59-8c442ef21636
📒 Files selected for processing (5)
job/controllers.gojob/job.gojob/job_test.gojob/singleton.gotests/job_test.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
resolves: flanksource/config-db#2300
Config-db scraper jobs can be launched manually and by schedule through separate duty Job instances.
Singleton previously used a mutex stored on the Job struct, so equal logical jobs could overlap when represented by different objects.
Use an in-process lock registry keyed by
Job.ID(), the single computed job identity. Resource-backed jobs use theName::ResourceType/ResourceIDformat; jobs without a resource useName. Replace the old stored alias field withAliases, which is only used for property lookup and display naming.GetPropertyandGetPropertyIntkeep their legacy lookup semantics:jobs.<name>.<property>wins over alias-specific values, and these accessors do not inherit the globaljobs.<property>fallback.Summary by CodeRabbit
New Features
Bug Fixes
Tests