Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/storage/version/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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) => {
Expand Down
11 changes: 7 additions & 4 deletions test/storage/object/conditionals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
});
});

Expand Down
30 changes: 17 additions & 13 deletions test/storage/version/audit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 = [];
Expand Down Expand Up @@ -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],
Expand All @@ -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)`,
);
});

Expand Down
Loading