fix(webapp): retry the snapshot sweep lock release so a transient Redis fault does not stall sweeps

The sweep lock release is a write, so the same Redis brownout that fails a
sweep pass also fails the release, and a single best-effort attempt left the
lock held for its full TTL (sweep budget plus two hours). One transient blip
could pause orphan sweeping across the fleet for hours. The release now retries
over a short bounded window, so a Redis that recovers within seconds frees the
lock promptly; the TTL remains only as the last-resort backstop.

Author: d-cs

apps/webapp/app/v3/snapshotSweepRunner.server.ts

@@ -13,13 +13,47 @@ end
 return 0
 `;
 
+// The release is a WRITE, so the fault that fails a pass fails the release too; a single attempt
+// then leaks the lock to its full TTL (budget + 2h), stalling every sweep for hours after a blip.
+// Retry over a short window so a Redis that recovers frees the lock promptly; the TTL is the backstop.
+const DEFAULT_RELEASE_RETRY_DELAYS_MS = [250, 500, 1000, 2000, 4000, 8000];
+
+const sleep = (ms: number) =>
+  ms > 0 ? new Promise<void>((resolve) => setTimeout(resolve, ms)) : Promise.resolve();
+
 type SweepCounts = Record<string, number | boolean>;
 
+async function releaseLock(
+  client: RedisClient,
+  fence: string,
+  retryDelaysMs: number[]
+): Promise<void> {
+  // attempts = retryDelaysMs.length + 1. A successful eval ends it, whether it deleted our lock
+  // (returned 1) or found the fence no longer ours (returned 0) — both mean nothing more to do.
+  for (let attempt = 0; ; attempt++) {
+    try {
+      await client.eval(RELEASE_LUA, 1, LOCK_KEY, fence);
+      return;
+    } catch (error) {
+      if (attempt >= retryDelaysMs.length) {
+        logger.warn(
+          "snapshot sweep lock release failed after retries; lock will clear on its TTL",
+          { error }
+        );
+        return;
+      }
+      await sleep(retryDelaysMs[attempt]);
+    }
+  }
+}
+
 export function buildSnapshotSweepRunner(deps: {
   client: RedisClient;
   sweep: (opts: { deadline: number; signal: AbortSignal }) => Promise<SweepCounts>;
   lockTtlMs: number;
   fence?: () => string;
+  /** Backoff between release attempts. Defaults to a bounded ~16s ramp; tests pass short delays. */
+  releaseRetryDelaysMs?: number[];
 }): SweepRunner {
   return async ({ deadline, signal }): Promise<SweepPassOutcome> => {
     // enqueueOnce gives no overlap protection: its dedup record IS the queue item and the ack
@@ -50,9 +84,11 @@ export function buildSnapshotSweepRunner(deps: {
       logger.error("snapshot orphan sweep pass failed", { error });
       return { outcome: "failed" };
     } finally {
-      await deps.client
-        .eval(RELEASE_LUA, 1, LOCK_KEY, fence)
-        .catch((error) => logger.warn("snapshot sweep lock release failed", { error }));
+      await releaseLock(
+        deps.client,
+        fence,
+        deps.releaseRetryDelaysMs ?? DEFAULT_RELEASE_RETRY_DELAYS_MS
+      );
     }
   };
 }

apps/webapp/test/snapshotSweepRunner.test.ts

@@ -128,3 +128,58 @@ redisTest("reports aborted when shutdown cancels the pass", async ({ redisOption
     await client.quit();
   }
 });
+
+function flakyEvalClient(real: RedisClient, failEvalTimes: number): RedisClient {
+  let fails = 0;
+  return new Proxy(real, {
+    get(target, prop, receiver) {
+      if (prop === "eval") {
+        return (...args: unknown[]) => {
+          if (fails < failEvalTimes) {
+            fails += 1;
+            return Promise.reject(new Error("simulated release eval failure"));
+          }
+          return (target.eval as (...a: unknown[]) => unknown)(...args);
+        };
+      }
+      const value = Reflect.get(target, prop, receiver);
+      return typeof value === "function" ? value.bind(target) : value;
+    },
+  }) as unknown as RedisClient;
+}
+
+redisTest("retries a transiently-failing release and frees the lock", async ({ redisOptions }) => {
+  const real = createRedisClient(redisOptions);
+  const client = flakyEvalClient(real, 2); // first two release attempts fail, third succeeds
+  try {
+    const runner = buildSnapshotSweepRunner({
+      client,
+      sweep: async () => CLEAN,
+      lockTtlMs: 60_000,
+      releaseRetryDelaysMs: [0, 0, 0],
+    });
+
+    expect((await runner(opts())).outcome).toBe("completed");
+    expect(await real.get(LOCK_KEY)).toBeNull(); // released after the retries, not left to TTL
+  } finally {
+    await real.quit();
+  }
+});
+
+redisTest("leaves the lock to its TTL only when every release attempt fails", async ({ redisOptions }) => {
+  const real = createRedisClient(redisOptions);
+  const client = flakyEvalClient(real, 999); // release can never succeed
+  try {
+    const runner = buildSnapshotSweepRunner({
+      client,
+      sweep: async () => CLEAN,
+      lockTtlMs: 60_000,
+      releaseRetryDelaysMs: [0],
+    });
+
+    expect((await runner(opts())).outcome).toBe("completed"); // runner never throws on a release failure
+    expect(await real.get(LOCK_KEY)).not.toBeNull(); // still held; the TTL is the backstop
+  } finally {
+    await real.quit();
+  }
+});