Green ticks for three hours: the lock whose TTL was the job's worst case

#redis #node #devops #architecture

Munchable runs curation jobs that spend money on model calls and write curated data, so two of them must never run at once. The lock is a Redis key set with NX and a TTL. That is the standard pattern, and the standard pattern has a parameter that is easy to reason about wrongly. Here is the outage it caused, the fix, and the test that keeps the two numbers honest.

The reasoning that felt right

The lock must outlive the run, or a slow job loses it mid-way and a second runner starts on top of it. So the TTL was the job's worst-case runtime: three hours on the scheduled runner, six from the command line, passed in by every caller.

export async function withCronLock<T>(
  name: string,
  ttlSeconds: number,
  fn: () => Promise<T>,
): Promise<CronLockResult<T>> {

That signature is the bug. It invites every caller to answer "how long may my job run?", and the honest answer to that question is the wrong value for a lock.

What happened

/**
 * It used to be the job's whole worst-case runtime, three hours on the
 * scheduled runner and six from the CLI, on the reasoning that the lock must
 * outlive the run. It does, but the cost landed somewhere else: a run that is
 * cancelled or killed never reaches its `finally`, so the key sat there for
 * the full three hours while the fifteen-minute schedule kept firing, and
 * every one of those runs skipped with `{ skipped: true, reason: 'locked' }`,
 * which the platform records as a SUCCESSFUL run. The dashboard showed green
 * every quarter hour while nothing was curated for three hours. Observed
 * exactly that on 2026-09-14, after two runs were cancelled by hand.
 */

Two runs cancelled by hand at about 12:15 and 12:24 UTC. Curation dead until 15:15. Twelve green ticks in the dashboard. Nothing in any log said why, because "locked" is a normal outcome and was reported as a successful skip.

The fix: a lease, not a timeout

The lock is now a short lease that the running job renews. A live job holds it for as long as it likes. A dead job loses it within the lease window.

export const LOCK_LEASE_S = 300;
/** Renewed comfortably inside the lease, so one failed renewal is survivable. */
export const RENEW_EVERY_MS = 90_000;

export async function withCronLock<T>(name: string, fn: () => Promise<T>): Promise<CronLockResult<T>> {
  const client = redis;
  if (!client) return { ran: false, reason: 'lock_unavailable' };
  const key = `cron:${name}`;
  const acquired = await client.set(key, String(Date.now()), { nx: true, ex: LOCK_LEASE_S });
  if (acquired !== 'OK') return { ran: false, reason: 'locked' };

  // Renew while the work runs. unref'd so a pending renewal can never be the
  // reason a worker process stays alive after the job is done.
  const renew = setInterval(() => {
    void client.expire(key, LOCK_LEASE_S).catch(() => {
      // A blip is survivable: the next tick renews, and the lease is three
      // ticks long. Losing the lock entirely only risks an overlap, which is
      // the lesser failure against curation stopping for hours.
    });
  }, RENEW_EVERY_MS);
  renew.unref?.();

  try {
    return { ran: true, result: await fn() };
  } finally {
    clearInterval(renew);
    try { await client.del(key); } catch { /* the lease releases it within five minutes */ }
  }
}

Three small things in there are load-bearing. The ttlSeconds parameter is gone from the signature entirely; a lease length exists as a test-only option, and the doc comment says callers should not pass it. The Redis client is captured into a local before the interval closure, so a module binding that could become null is never re-read from inside the timer. And the renewal failure is swallowed with a stated reason: losing the lock risks an overlap, which is the lesser failure against curation stopping for hours.

The test that ties the two numbers together

Neither constant means anything on its own. The lease has to cover a missed renewal or a Redis blip drops the lock. It also has to be short or a dead run blocks the schedule.

test('the cron lock renews well inside its own lease', () => {
  const leaseMs = LOCK_LEASE_S * 1000;
  assert.ok(leaseMs >= RENEW_EVERY_MS * 3,
    `lease ${leaseMs}ms must cover at least three renewal ticks of ${RENEW_EVERY_MS}ms`);
  // And short enough that a dead run costs at most one scheduled cycle. The
  // curation jobs run every 15 minutes.
  assert.ok(leaseMs <= 15 * 60 * 1000, 'a dead run must not block more than one 15-minute cycle');
});

Five minutes against ninety-second ticks is 3.3 ticks, just inside. Anyone who later doubles the tick interval without touching the lease gets a failing test with the reason in its name.

The runbook line it produced

The docs now say: if curation ever looks idle, check cron:<job> in Redis first. That sentence would have saved three hours.

There is a coda. Two days after this fix, the scheduled runner was deleted entirely and curation became an operator-run command with a person watching it. The lock is still taken, so two terminals cannot write at once, and the lease is what makes a killed terminal harmless. That story is its own post. What the jobs actually do is covered in AI proposes, the engine disposes, and the result of them is every ingredient answer on munchable.app/answers.

DEV Community