[High] Contest sync cron job lacks multi-instance execution guard (duplicate execution risk)

[coderabbitai]

Summary

Reported by CodeRabbit during review of PR #269 (comment: #269 (comment)). Requested by @kunalverma2512.

startContestSyncJob() (server/jobs/contestSync.js) is invoked once from server/server.js on every process boot, and schedules an hourly node-cron job (0 * * * *) that calls ContestService.syncCodeforcesContests() in-process, with no coordination mechanism between multiple running processes.

Why this matters (the real problem)

node-cron schedules are purely local to the Node.js process that registers them. There is no built-in distributed locking, leader election, or "only one instance runs this" semantics — each process that calls startContestSyncJob() will independently fire its own timer and execute the sync logic at the top of every hour, completely unaware that other instances exist.

Today, with a single server instance, this is invisible — there's only one cron firing, so it looks correct. But this is a latent architectural landmine: the moment this application is horizontally scaled (e.g., deployed behind a load balancer with 2+ Render/Railway/Heroku dynos, run under PM2 cluster mode, or moved to Kubernetes with replicas > 1), every single instance will independently run the full sync at the same wall-clock minute, with zero awareness of the others.

What happens without this fix

• With N running instances, the Codeforces contest.list API gets hit N times within the same minute, every hour, instead of once. Codeforces is a public, informally rate-limited API used by thousands of tools; unnecessary duplicate traffic is poor API citizenship and increases the chance of throttling/blocking for this application specifically.
• ContestRepository.bulkUpsertContests() and ContestRepository.pruneStaleReminders() run N times concurrently against MongoDB for the same batch of documents. The upserts themselves are idempotent (keyed on the {platform, contestId} unique index), so this will not corrupt data — but it multiplies write load, connection pool usage, and log noise linearly with instance count, for zero additional benefit.
• Debugging becomes harder: logs from runSync (Synced ${count} Codeforces contests / Contest sync failed:) will appear N times per hour across different instances, making it look like sync is running more often than intended, and complicating root-cause analysis of any future sync-related bug.
• This is exactly the kind of issue that is cheap to fix now (while the codebase is small and the feature is new) and expensive to diagnose later (once it's live in production, scaled, and someone is confused about why the DB write count "doesn't add up").

How this resolves the issue / where it improves the codebase

Introducing a distributed lock ensures exactly one instance executes the sync per scheduled interval, regardless of how many processes/instances/dynos are running the app. This is a standard, well-understood pattern in backend engineering for any "scheduled job + horizontally scalable service" combination — the kind of thing that is expected in any production-grade Node.js/Express + MongoDB service that uses in-process cron scheduling instead of an external job scheduler.

Recommended approaches (pick one, roughly in order of simplicity vs. robustness)

1. MongoDB-based leader lock (simplest, no new infra)

   • Create a tiny SyncLock collection with a single document (or one per job name), and use findOneAndUpdate with a condition like { lockedUntil: { $lt: now } } to atomically "acquire" the lock and set a short expiry (e.g., 5 minutes). Only the instance whose findOneAndUpdate succeeds proceeds to run syncCodeforcesContests(); all others skip that tick.
   • This requires no new infrastructure since MongoDB is already a dependency, and atomic findOneAndUpdate guarantees correctness under concurrent execution.

2. TTL-based Mongo lock document

   • Similar to above, but rely on a MongoDB TTL index to auto-expire the lock document instead of manually checking lockedUntil, simplifying cleanup logic.

3. Move to an external scheduler

   • If the team anticipates scaling soon, consider moving this sync out of the web process entirely into a dedicated worker/cron service (e.g., a separate lightweight Node process, a scheduled serverless function, or a platform-native cron feature like Render Cron Jobs / GitHub Actions scheduled workflow that hits an internal POST /api/contests/sync admin-only endpoint). This decouples "keep the API responsive" from "run background jobs," which is generally the better long-term architecture once traffic grows.

Example sketch (Mongo lock approach)

// server/models/SyncLock.js
const SyncLockSchema = new mongoose.Schema({
  jobName: { type: String, unique: true, required: true },
  lockedUntil: { type: Date, required: true },
});

// server/jobs/contestSync.js
const acquireLock = async (jobName, ttlMs) => {
  const now = new Date();
  const result = await SyncLock.findOneAndUpdate(
    { jobName, lockedUntil: { $lt: now } },
    { jobName, lockedUntil: new Date(now.getTime() + ttlMs) },
    { upsert: true, new: true }
  );
  return result !== null; // simplistic; refine for race edge cases with findOneAndUpdate atomicity guarantees
};

Affected files

• server/jobs/contestSync.js
• server/server.js
• (new) a lock model/collection, e.g. server/models/SyncLock.js

Acceptance criteria

• Running 2+ instances of the server concurrently results in exactly one syncCodeforcesContests() execution per scheduled hourly tick (verifiable via logs/lock document state).
• Single-instance behavior (today's deployment) remains unchanged in outcome.
• Lock acquisition failure (e.g., DB unreachable) degrades gracefully — sync is skipped for that tick with a logged warning, not a crash.

Concepts/topics to research before working on this (learning goals)

• Distributed locking / leader election patterns — why setInterval/node-cron alone is insufficient once you have more than one process, and how atomic database operations (findOneAndUpdate, compare-and-swap semantics) can emulate a lock without external infra like Redis/ZooKeeper.
• MongoDB atomic operations — understanding why findOneAndUpdate (a single atomic server-side operation) is safe under concurrency, versus a findOne followed by a separate updateOne (which is a classic read-then-write race condition).
• TTL indexes in MongoDB — how expireAfterSeconds works and how it can be used for automatic lock expiry/cleanup instead of manual bookkeeping.
• The difference between horizontal scaling and vertical scaling, and why "it works with 1 instance" is not the same as "it works in production," especially for anything involving timers, in-memory state, or schedulers.
• Idempotency in backend design — study why ContestRepository.bulkUpsertContests being idempotent (via the unique compound index) is what prevented this bug from being a data corruption issue rather than just a wasted-work issue; this is a core industry principle for anything that might run more than once (retries, at-least-once delivery, duplicate cron ticks, etc.).
• Job scheduling architecture in real systems — read about how companies typically separate "web/API servers" from "background job/worker processes" (e.g., BullMQ + Redis, Sidekiq-style architectures, dedicated cron services), and when it makes sense to graduate from in-process cron to a dedicated scheduler.

References

• PR: feat: upcoming contests tracker for Codeforces with in-app reminders  #269
• Review comment: feat: upcoming contests tracker for Codeforces with in-app reminders  #269 (comment)
• Requested by: @kunalverma2512

[ida-jemi]

@kunalverma2512 I've raised a PR for this issue.
PR: #280

[kunalverma2512]

@ida-jemi I'll review soon as currently occupied and busy but appreciated your work
Kindly wait untill I respond