Skip to content

Commit a73bfcd

Browse files
carderneTrigger.dev RepoOps
authored andcommitted
test(run-store): stabilize recovery lease takeover test
Remove the timing race from the recovery-lease takeover test by holding the first partition behind an explicit promise until the successor acquires the expired lease. Start the expiry wait only after the first partition begins, and always finish the sweep before closing Redis, including when an assertion fails. The four recovery-lease tests passed in 10 consecutive runs. No production behavior changes. Mono-RevId: e7706d88f32abb7a23b560ed310266104997de48
1 parent 3aa9aa6 commit a73bfcd

1 file changed

Lines changed: 19 additions & 8 deletions

File tree

‎internal-packages/run-store/src/redisSnapshotStore.recoveryLease.test.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,14 +58,18 @@ describe("RedisSnapshotStore recovery lease", () => {
5858
async ({ redisOptions }) => {
5959
const store = new RedisSnapshotStore({ redisOptions, completedTtlMs: 60_000 });
6060
const ttlMs = 200;
61+
const expiryMarginMs = 50;
6162
const scanned: number[] = [];
62-
// A slow partition worker (300ms > the 200ms lease TTL) so the sweep outlives the lease; the
63-
// pendingIndex is a no-op double. Only the lease (real Redis) is under test here.
63+
let markPartitionStarted!: () => void;
64+
const partitionStarted = new Promise<void>((resolve) => (markPartitionStarted = resolve));
65+
let releasePartition!: () => void;
66+
const partitionGate = new Promise<void>((resolve) => (releasePartition = resolve));
6467
const sweeperA = new RecoverySweeper({
6568
worker: {
6669
processPartition: async (partition) => {
6770
scanned.push(partition);
68-
await sleep(300);
71+
markPartitionStarted();
72+
await partitionGate;
6973
return [];
7074
},
7175
},
@@ -77,20 +81,27 @@ describe("RedisSnapshotStore recovery lease", () => {
7781
partitionCount: 5,
7882
acquireTick: async () => store.acquireOrRenewRecoveryLease("owner-A", ttlMs),
7983
});
84+
const aTick = sweeperA.tick();
8085
try {
81-
// A renews before partition 0, then processes it for 300ms, outliving its 200ms lease.
82-
const aTick = sweeperA.tick();
83-
// Mid partition-0, after A's lease has expired, a successor takes over.
84-
await sleep(250);
86+
await Promise.race([partitionStarted, aTick]);
87+
expect(scanned).toEqual([0]);
88+
// Start the expiry wait only after Redis granted A's lease, and hold partition 0 until B owns it.
89+
await sleep(ttlMs + expiryMarginMs);
8590
expect(await store.acquireOrRenewRecoveryLease("owner-B", 10_000)).toBe(true);
8691
// A finishes partition 0, its renew before partition 1 fails, and it stops there.
92+
releasePartition();
8793
const result = await aTick;
8894
expect(scanned).toEqual([0]); // A processed ONLY the in-flight partition, never advanced
8995
expect(result.skipped).toBe(false); // it did work before losing the lease
9096
// The successor still holds the lease; A cannot reclaim it while B renews.
9197
expect(await store.acquireOrRenewRecoveryLease("owner-A", ttlMs)).toBe(false);
9298
} finally {
93-
await store.quit();
99+
releasePartition();
100+
try {
101+
await aTick;
102+
} finally {
103+
await store.quit();
104+
}
94105
}
95106
}
96107
);

0 commit comments

Comments
 (0)