feat: harden contest sync lease with safe release, overlap guard, tes…

server/jobs/README.md

@@ -0,0 +1,38 @@
+# Scheduled Jobs
+
+## Contest sync (`contestSync.js`)
+
+**Strategy:** Database-backed lease (MongoDB), not a dedicated worker process or queue.
+
+Every backend instance calls `startContestSyncJob()` on boot. Each instance
+independently registers the same hourly `node-cron` schedule, but before
+running the actual sync, an instance must atomically acquire a lease in the
+`synclocks` collection (`SyncLock` model, `jobName: "contestSync"`).
+
+- Only the instance holding the lease runs `syncCodeforcesContests()`.
+- The lease is released immediately after the run finishes (success or
+  failure) so the next tick isn't blocked waiting on the full TTL.
+- If a holder crashes or hangs, the lease auto-expires after `LOCK_TTL_MS`
+  (currently 10 minutes) and the next tick on another instance can take over.
+- A per-instance in-memory flag additionally prevents the *same* instance
+  from starting an overlapping sync if one run takes longer than expected.
+
+**Deployment implication:** no special configuration is needed to run this
+app with multiple replicas (PM2 cluster mode, container replicas, rolling
+deploys, multiple dynos) — all instances share one MongoDB and coordinate
+through it automatically.
+
+**If this ever needs to change:** if sync duration grows significantly, or
+side effects become non-idempotent (emails, push notifications), consider
+moving to a dedicated worker/scheduler process (see issue #277, Option 1)
+so web instances are no longer cron owners at all.
+
+## Local validation
+
+To manually confirm the lease behaves correctly with multiple instances:
+1. Run `node server.js` in two terminals against the same MongoDB.
+2. Confirm one logs `Acquired lease ... starting sync` and the other logs
+   `Skipped ... another instance holds the lease`.
+3. Check the `synclocks` collection has exactly one document for
+   `jobName: "contestSync"`.
+

server/jobs/contestSync.test.js

@@ -0,0 +1,80 @@
+import { test, describe, mock, beforeEach } from "node:test";
+import assert from "node:assert/strict";
+import SyncLock from "../models/SyncLock.js";
+import { acquireLock, releaseLock } from "./contestSync.js";
+
+describe("contestSync distributed lock", () => {
+  beforeEach(() => {
+    mock.restoreAll();
+  });
+
+  test("acquires the lock when no unexpired lease exists", async () => {
+    mock.method(SyncLock, "findOneAndUpdate", async (filter, update) => ({
+      jobName: update.jobName,
+      ownerId: update.ownerId,
+      lockedUntil: update.lockedUntil,
+    }));
+
+    const acquired = await acquireLock("contestSync", 60000, "instanceA:run1");
+    assert.equal(acquired, true);
+  });
+
+  test("only one of two concurrent acquirers wins (duplicate key race)", async () => {
+    let firstCallWon = false;
+    mock.method(SyncLock, "findOneAndUpdate", async (filter, update) => {
+      if (!firstCallWon) {
+        firstCallWon = true;
+        return { jobName: update.jobName, ownerId: update.ownerId, lockedUntil: update.lockedUntil };
+      }
+      const err = new Error("E11000 duplicate key error");
+      err.code = 11000;
+      throw err;
+    });
+
+    const [a, b] = await Promise.all([
+      acquireLock("contestSync", 60000, "instanceA:run1"),
+      acquireLock("contestSync", 60000, "instanceB:run1"),
+    ]);
+
+    assert.equal([a, b].filter(Boolean).length, 1);
+  });
+
+  test("returns false and does not throw when the DB is unreachable", async () => {
+    mock.method(SyncLock, "findOneAndUpdate", async () => {
+      throw new Error("connection timed out");
+    });
+
+    const acquired = await acquireLock("contestSync", 60000, "instanceA:run1");
+    assert.equal(acquired, false);
+  });
+
+  test("a new owner can acquire once the previous lease has expired", async () => {
+    mock.method(SyncLock, "findOneAndUpdate", async (filter, update) => ({
+      jobName: update.jobName,
+      ownerId: update.ownerId,
+      lockedUntil: update.lockedUntil,
+    }));
+
+    const acquired = await acquireLock("contestSync", 60000, "instanceB:run2");
+    assert.equal(acquired, true);
+  });
+
+  test("releaseLock deletes scoped to jobName + ownerId only", async () => {
+    let deleteFilter;
+    mock.method(SyncLock, "deleteOne", async (filter) => {
+      deleteFilter = filter;
+      return { deletedCount: 1 };
+    });
+
+    await releaseLock("contestSync", "instanceA:run1");
+    assert.deepEqual(deleteFilter, { jobName: "contestSync", ownerId: "instanceA:run1" });
+  });
+
+  test("releaseLock does not throw when deleteOne fails", async () => {
+    mock.method(SyncLock, "deleteOne", async () => {
+      throw new Error("connection lost");
+    });
+
+    await assert.doesNotReject(releaseLock("contestSync", "instanceA:run1"));
+  });
+});

server/package.json

@@ -6,7 +6,7 @@
   "scripts": {
     "start": "node server.js",
     "dev": "nodemon server.js",
-    "test": "echo \"Error: no test specified\" && exit 1"
+    "test": "node --test"
   },
   "keywords": [],
   "author": "",