feat: harden contest sync lease with safe release, overlap guard, tes… - #1
Closed
ida-jemi wants to merge 2 commits into
Closed
feat: harden contest sync lease with safe release, overlap guard, tes…#1ida-jemi wants to merge 2 commits into
ida-jemi wants to merge 2 commits into
Conversation
…distributed-lock fix: add MongoDB-based distributed lock to contest sync cron job
…ts & docs - SyncLock now tracks ownerId so releases are scoped to the acquiring instance and can never delete another instance's active lease. - Lease is released immediately after each run instead of relying solely on TTL expiry, reducing unnecessary wait time between ticks. - Added an in-process isRunning guard so a slow sync cannot overlap with the next scheduled tick on the same instance. - Logs now include a run ID and owner ID, and distinguish skipped vs failed vs successful runs. - Added automated tests (node:test) covering concurrent acquisition, DB failure, lease handoff, and scoped release. - Documented the chosen strategy in server/jobs/README.md. Closes kunalverma2512#277
ida-jemi
force-pushed
the
feat/contest-sync-lease-hardening
branch
from
July 23, 2026 09:15
40f1915 to
1573ec2
Compare
Owner
Author
|
Closing, this was a staging PR before kunalverma2512#280 merged upstream. kunalverma2512#277's work is now being opened directly against kunalverma2512/CodeLens:main here: kunalverma2512#284 |
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
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.
📌 Pull Request Summary
🔗 Related Issue
Closes kunalverma2512#277
📝 Description
Changes Made
ownerIdto theSyncLockmodel so a lease is tagged with exactly which instance/run acquired it, allowing safe, scoped release that can never delete another instance's active lease.server/jobs/contestSync.jsto release the lease immediately after each run completes (success or failure) instead of relying solely on TTL expiry, so the next legitimate run doesn't wait unnecessarily.isRunningguard so a single instance cannot start an overlapping sync if a previous run is still in flight when the next scheduled tick fires.server/jobs/contestSync.test.js, using Node's built-innode:testrunner) covering concurrent lock acquisition, DB-unreachable degradation, lease handoff after expiry, and owner-scoped release.server/jobs/README.md.npm test(node --test) since the repo had no test runner configured yet.Motivation
This is a follow-up to kunalverma2512#276 covering the full High-severity finding from the review of kunalverma2512#269: an in-process cron scheduler has no leader-election or distributed-lock mechanism, so every backend replica independently runs its own schedule. kunalverma2512#276 introduced a basic TTL-based lock, but this issue requires the harder production-safety guarantees called out in its acceptance criteria, safe lock release scoped to the correct owner, protection against overlapping runs on the same instance, recovery from a crashed lock holder, and test/documentation coverage, none of which kunalverma2512#276 addressed on its own. This PR closes those gaps so the contest sync job is safe to run across any number of replicas without multiplying external API calls, DB writes, or log noise.
🚀 Type of Change
Select all that apply:
🧪 Testing
Verification
Test Details
node --test, covering: successful lock acquisition, concurrent acquirers racing for the same lease (only one wins via duplicate-key handling), gracefulfalsereturn (no throw) when the DB is unreachable, reacquisition after lease handoff, owner-scopedreleaseLockdeletion, andreleaseLocknot throwing on DB failure. All 6 pass.node server.jsinstances against the same MongoDB within that window. Instance 1 loggedAcquired lease ... starting sync→synced 24 contest(s); Instance 2 logged onlySkipped ... another instance holds the lease.with no acquire/sync lines, confirming exclusivity under real concurrency. Delay was removed before committing.synclockscollection holds exactly one document forjobName: "contestSync", and that it is removed immediately after a successful run rather than persisting for the full TTL window.📸 Screenshots / Demo (If Applicable)
Terminal 1 & 2:
npm test:
✅ Checklist
📚 Additional Notes
This PR is based on top of the
fix/contest-sync-distributed-lockbranch from kunalverma2512#276 rather thanmain, since it extends that work directly, please merge kunalverma2512#276 first (or review this PR's diff against that branch) to avoid seeing unrelated changes. The chosen approach is a MongoDB-backed lease (Option 2 from the issue) rather than a dedicated worker process or queue, since it requires no new infrastructure; the tradeoffs and a migration path to a dedicated worker are documented inserver/jobs/README.mdfor future reference.