fix(sync): only reclaim orphaned snapshot on confirmed rev-mismatch (#9040)

The loser-orphan cleanup deleted the just-written immutable snapshot on ANY
commit failure. A rev-mismatch is a confirmed server rejection (ops did not
commit), but a network/5xx error is ambiguous — the ops PUT may have landed and
committed, in which case the snapshot is still referenced and deleting it would
strand a reader. Restrict the cleanup to UploadRevToMatchMismatchAPIError.

Adds a test asserting the immutable snapshot survives a non-mismatch commit
failure. Full op-log suite (3428) green.
This commit is contained in:
Johannes Millan 2026-07-15 14:53:46 +02:00
parent 54bc6d4800
commit 0ae073ca42
2 changed files with 35 additions and 6 deletions

View file

@ -2163,12 +2163,18 @@ export class FileBasedSyncAdapterService {
opsRev,
));
} catch (e) {
// #9040: our ops never committed — a concurrent compactor won the ops race.
// The immutable snapshot we wrote this compaction is now an orphan no
// committed ops file references, so reclaim it before bubbling up. (Only
// when we compacted; otherwise snapshotRef.file is the still-referenced
// predecessor and must NOT be deleted.)
if (needsCompaction && snapshotRef.file) {
// #9040: reclaim the immutable snapshot we just wrote — but ONLY on a
// confirmed rev-mismatch rejection, which proves the server refused our ops
// PUT, so a concurrent compactor won and our snapshot is a true orphan. Any
// other error (network/5xx) is AMBIGUOUS: the PUT may have landed and
// committed, in which case the snapshot is still referenced and MUST survive
// (readers would otherwise strand on it). Deleting only applies when we
// compacted; otherwise snapshotRef.file is the still-referenced predecessor.
if (
e instanceof UploadRevToMatchMismatchAPIError &&
needsCompaction &&
snapshotRef.file
) {
await this._removeGenStateFile(provider, snapshotRef.file);
}
throw e;

View file

@ -183,4 +183,27 @@ describe('File-Based Sync Integration - Concurrent Split Compaction (#9040)', ()
expect(provider.hasFile('sync-state__1__winner-client.json')).toBe(true);
expect(provider.hasFile('sync-state__1__loser-client.json')).toBe(false);
});
it('ambiguous (non-mismatch) commit failure keeps the immutable snapshot', async () => {
const provider = harness.getProvider();
await seedFolderAtCap();
// A compactor writes its immutable snapshot, then its ops commit fails with a
// NON-mismatch error (e.g. a dropped connection). That is ambiguous — the PUT
// may have landed and committed — so the snapshot MUST NOT be reclaimed, or a
// reader of the committed ops file would strand on it.
const client = harness.createClient('netfail-client');
const realUploadFile = provider.uploadFile.bind(provider);
spyOn(provider, 'uploadFile').and.callFake(
async (path: string, data: string, rev: string | null, isForce?: boolean) => {
if (path === C.OPS_FILE) throw new Error('simulated network failure');
return realUploadFile(path, data, rev, isForce);
},
);
await expectAsync(client.uploadOps([addTaskOp(client, 'netfail-op')])).toBeRejected();
// syncVersion 3 (seed 1 → fill 2 → this compaction 3): snapshot must remain.
expect(provider.hasFile('sync-state__3__netfail-client.json')).toBe(true);
});
});