-
Notifications
You must be signed in to change notification settings - Fork 2
Prevent concurrent syncs #27
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces concurrency control to prevent multiple syncs from running simultaneously by adding a simple flag-based check that skips overlapping sync requests rather than blocking them. This design choice helps maintain synchronized serial numbers across multiple instances running on cron schedules.
Key Changes:
- Added skip-on-busy concurrency control using an
isRunningflag inSyncService - Introduced a new metric counter to track skipped syncs when the system is running slowly
- Refactored sync logic by extracting the main body into a new
doSync()method
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/main/java/net/ripe/rpki/rsyncit/service/SyncService.java |
Implements concurrency control with an isRunning flag check and extracts sync logic to a new synchronized doSync() method |
src/main/java/net/ripe/rpki/rsyncit/rrdp/RRDPFetcherMetrics.java |
Adds tooSlow counter metric to track when syncs are skipped due to concurrent execution, plus formatting improvements |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This one is to skip concurrent syncs. Skipping a sync instead of blocking and waiting is intentional here because we still want two (multiple) instances syncing at more or less the same time based on cron mask. Blocking will more likely cause serials on these instances to diverge (I guess).