diff --git a/internal-packages/run-engine/src/run-queue/index.ts b/internal-packages/run-engine/src/run-queue/index.ts index 9c8415e07..c2823de8a 100644 --- a/internal-packages/run-engine/src/run-queue/index.ts +++ b/internal-packages/run-engine/src/run-queue/index.ts @@ -5747,12 +5747,9 @@ if gatedPending ~= nil then end if redis.call('ZADD', unpack(gatedArgs)) > 0 then for _, ckQueueName in ipairs(gatedPending) do - -- Only the members this call put at the floor may take their parked tag back. The - -- batched ZADD reports how many it added but not which, and gatedPending can hold an - -- already-registered variant whenever the pass-1 scan was truncated, so the current - -- score is the discriminator: at the floor means it either just registered or has no - -- credit to lose either way. Without this the parked tag overwrites a live advanced - -- one and rewinds that variant's clock, which is the one thing NX exists to stop. + -- The batch ZADD reports how many it added, not which, and gatedPending can hold an + -- already-registered variant when the pass-1 scan was truncated. At the floor is the + -- discriminator: anything above it has spent credit that must not be rewound. local gateCur = redis.call('ZSCORE', ckVtimeKey, ckQueueName) if gateCur and tonumber(gateCur) <= floor then local gateIdle = redis.call('ZSCORE', ckVtimeIdleKey, ckQueueName) diff --git a/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts b/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts index f50317494..ef5334a3a 100644 --- a/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts +++ b/internal-packages/run-engine/src/run-queue/tests/ckVtimeGatedRewind.test.ts @@ -133,4 +133,55 @@ describe("CK vtime: the gated batch must not rewind a live tag", () => { await queue.quit(); } }); + + redisTest( + "a variant registering here still gets its parked tag back", + async ({ redisContainer }) => { + // The other half of the same branch. Deleting the idle correction outright used to + // leave every vtime suite green, so the rewind guard was pinned while the behaviour it + // guards was not. + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + const cks = ["returner", "aa", "bb"]; + for (let i = 0; i < cks.length; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${cks[i]}`, + concurrencyKey: cks[i], + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const returner = variantName("returner"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(returner); + const ckVtimeIdleKey = testOptions.keys.ckVtimeIdleKeyFromQueue(returner); + + await queue.redis.zadd(ckVtimeKey, 0, variantName("aa"), 1, variantName("bb")); + await queue.redis.zrem(ckVtimeKey, returner); + await queue.redis.zadd(ckVtimeIdleKey, 5, returner); + + await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, QUEUE, 1); + for (const ck of cks) { + await queue.redis.sadd( + testOptions.keys.queueCurrentConcurrencyKeyFromQueue(variantName(ck)), + "occupant" + ); + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 1); + + // Registering at the floor instead would hand it a turn it already spent. + expect(await queue.redis.zscore(ckVtimeKey, returner)).toBe("5"); + } finally { + await queue.quit(); + } + } + ); });