diff --git a/src/storage/version/audit.js b/src/storage/version/audit.js index c876218b..c8964249 100644 --- a/src/storage/version/audit.js +++ b/src/storage/version/audit.js @@ -191,14 +191,14 @@ function usersNormalized(usersJson) { * and within AUDIT_TIME_WINDOW_MS and both last and new are edits (no version), replace that * line; else append. A version entry always appends and is never replaced (breaks the window). * - * Uses If-Match on the PUT so that a concurrent write causes a 412, which triggers up to 4 - * retries with random jitter to reduce thundering-herd contention (5 total attempts). + * Uses If-Match on the PUT so that a concurrent write causes a 412, which triggers up to 6 + * retries with random jitter to reduce thundering-herd contention (7 total attempts). * @param {object} env * @param {{ bucket: string, org: string }} ctx - bucket, org * @param {string} repo * @param {string} fileId * @param {object} entry - { timestamp, users, path, versionLabel?, versionId? } - * @param {number} [attempt=0] - retry counter (max 4 retries) + * @param {number} [attempt=0] - retry counter (max 6 retries) * @returns {Promise<{ status: number }>} */ export async function writeAuditEntry(env, ctx, repo, fileId, entry, attempt = 0) { @@ -269,10 +269,13 @@ export async function writeAuditEntry(env, ctx, repo, fileId, entry, attempt = 0 const resp = await client.send(new PutObjectCommand(putInput)); return { status: resp?.$metadata?.httpStatusCode ?? 200 }; } catch (e) { - if (e?.$metadata?.httpStatusCode === 412 && attempt < 4) { - // Exponential jitter: 0-50, 0-100, 0-200, 0-400 ms worst-case (~750 ms total). - // Grown from linear (~500 ms total) after PreconditionFailed burst of 5011/24h - // on 2026-05-12. Retry count unchanged. See COR-26. + if (e?.$metadata?.httpStatusCode === 412 && attempt < 6) { + // Exponential jitter, 6 retries: per-attempt max 50, 100, 200, + // 400, 800, 1600 ms (~3050 ms worst-case total). Two prior 4-retry + // backoff bumps did not converge because the contention window is + // wider than per-write latency — every retry inside a short window + // observes the same losing-etag generation. Append-only ledger + // (one object per entry) is the next escalation if this still fails. const delay = Math.random() * 50 * 2 ** attempt; // eslint-disable-next-line no-await-in-loop -- sequential retry with jitter await new Promise((r) => { diff --git a/test/storage/object/conditionals.test.js b/test/storage/object/conditionals.test.js index d5efe35e..fe4310c8 100644 --- a/test/storage/object/conditionals.test.js +++ b/test/storage/object/conditionals.test.js @@ -228,7 +228,10 @@ describe('Conditional Headers', () => { assert.strictEqual(resp.status, 200); }); - it('returns 412 when ETag does not match and does not retry', async () => { + it('returns 412 when ETag does not match and does not retry', async function returnsImmediateOn412NoRetry() { + // writeAuditEntry's 6-retry exponential jitter has a worst-case budget of + // ~3050 ms, which exceeds mocha's 2000 ms default; bound this test above it. + this.timeout(8000); const existingEtag = '"existing123"'; s3Mock .on(GetObjectCommand) @@ -260,10 +263,10 @@ describe('Conditional Headers', () => { clientConditionals, ); - // Should return 412 and NOT retry + // Should return 412 and NOT retry the main PUT assert.strictEqual(resp.status, 412); - // 5 audit PUT attempts (1 initial + 4 retries on 412) + 1 main PUT = 6 total - assert.strictEqual(s3Mock.commandCalls(PutObjectCommand).length, 6); + // 7 audit PUT attempts (1 initial + 6 retries on 412) + 1 main PUT = 8 total + assert.strictEqual(s3Mock.commandCalls(PutObjectCommand).length, 8); }); }); diff --git a/test/storage/version/audit.test.js b/test/storage/version/audit.test.js index b01f42ff..f8d04ca0 100644 --- a/test/storage/version/audit.test.js +++ b/test/storage/version/audit.test.js @@ -814,7 +814,7 @@ describe('Version Audit', () => { assert.strictEqual(lines.length, 2, 'both entries must be present (no collapse across mismatched users)'); }); - it('returns status 500 when PUT 412 persists across all 5 attempts', async () => { + it('returns status 500 when PUT 412 persists across all 7 attempts', async () => { let getCallCount = 0; let putCallCount = 0; @@ -857,16 +857,16 @@ describe('Version Audit', () => { path: 'repo/doc.html', }); - assert.strictEqual(result.status, 500, 'persistent 412 must surface as 500 after 5 total attempts'); + assert.strictEqual(result.status, 500, 'persistent 412 must surface as 500 after 7 total attempts'); assert.strictEqual(result.error, 'precondition failed'); - assert.strictEqual(putCallCount, 5, 'must attempt PUT 5 times total (1 initial + 4 retries)'); - assert.strictEqual(getCallCount, 5, 'must re-read on each attempt'); + assert.strictEqual(putCallCount, 7, 'must attempt PUT 7 times total (1 initial + 6 retries)'); + assert.strictEqual(getCallCount, 7, 'must re-read on each attempt'); }); - it('uses exponential jitter backoff (0-50, 0-100, 0-200, 0-400 ms) across four 412 retries', async () => { + it('uses exponential jitter backoff (0-50, 0-100, 0-200, 0-400, 0-800, 0-1600 ms) across six 412 retries', async () => { // Stubs setTimeout and Math.random to capture per-retry delays. - // Asserts per-attempt exponential upper bounds and that total elapsed sleep - // exceeds the prior linear worst-case (500 ms). See COR-26. + // Asserts per-attempt exponential upper bounds for the 6-retry ladder + // (50, 100, 200, 400, 800, 1600 ms) and total elapsed sleep ~3050 ms. const originalSetTimeout = globalThis.setTimeout; const originalRandom = Math.random; const delays = []; @@ -919,8 +919,8 @@ describe('Version Audit', () => { Math.random = originalRandom; } - assert.strictEqual(delays.length, 4, 'must sleep between each of the 4 retries'); - const upperBounds = [50, 100, 200, 400]; + assert.strictEqual(delays.length, 6, 'must sleep between each of the 6 retries'); + const upperBounds = [50, 100, 200, 400, 800, 1600]; delays.forEach((ms, i) => { assert.ok( ms >= 0 && ms < upperBounds[i], @@ -935,14 +935,18 @@ describe('Version Audit', () => { delays[3] >= 200, `delay[3]=${delays[3]} must exceed the prior linear cap of 200 ms`, ); + assert.ok( + delays[5] >= 800, + `delay[5]=${delays[5]} must reach the new 6-retry exponential ceiling (>=800 ms)`, + ); const total = delays.reduce((a, b) => a + b, 0); assert.ok( - total > 500, - `total elapsed sleep ${total} must exceed prior linear worst-case (500 ms)`, + total > 1500, + `total elapsed sleep ${total} must exceed prior 4-retry worst-case (~750 ms)`, ); assert.ok( - total < 750, - `total elapsed sleep ${total} must stay within the new worst-case (~750 ms)`, + total < 3200, + `total elapsed sleep ${total} must stay within the new 6-retry worst-case (~3050 ms)`, ); });