feat(run-engine): fair virtual-time scheduling for the concurrency-key dequeue - #4367
feat(run-engine): fair virtual-time scheduling for the concurrency-key dequeue#43671stvamp wants to merge 40 commits into
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds opt-in CK virtual-time scheduling to the run queue. The feature introduces environment and engine configuration, queue key helpers, Redis Lua commands for fair enqueue, dequeue, and nack handling, virtual-time state with TTL and cleanup behavior, and updated Redis command typings. New integration suites cover ordering, fairness, batching, concurrency, registration, garbage collection, disabled-mode compatibility, and Redis command overhead. Design, rollout, limitation, and research documentation are also included. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1stvamp
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
bc6dc8d to
99eab9d
Compare
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
f90585f to
9b34c9a
Compare
9b34c9a to
0abcf84
Compare
|
Benchmark: flag OFF vs ON. Relative numbers from a single local homelab box (production-like multi-cluster topology, not production; not prod scale).
Full method, scenarios, and tables: results-2026-07-27.md |
0abcf84 to
71e71cc
Compare
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@1stvamp this looks great! The one thing I'd be curious about is how do these changes effect the following:
How do both of those grow/react to changes in cardinality (e.g. all of a sudden there is a queue with 10k different concurrency keys). |
@ericallam good question, I'll do some testing. |
073b881 to
5722cdc
Compare
|
@ericallam ran it on the homelab box (dedicated redis, flag OFF vs ON, relative numbers only). Short version: memory is linear in key count, CPU barely moves and doesn't grow with cardinality.
Full tables and method: results-cardinality-2026-07-28.md. Shout if you'd like a bigger cardinality or a longer window. |
ericallam
left a comment
ericallam
left a comment
There was a problem hiding this comment.
This all looks good! Just needs some cleaning up before we can merge. I think the whole e2e testing/bench thing shouldn't be included, along with the benchmark results and plan files and all that stuff. It should ideally be just the code and the changeset and the CI tests
💯 will do |
|
Stripped the e2e/bench harnesses, results, plans and design docs, so the branch is just the scheduler code, the CI tests and the server-changes note now. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
66a0d45 to
4c1fc25
Compare
Reframe the retained findings/research headers so they no longer say 'delete before merge' (the spike harness is archived and ships nothing; the findings ship as a design reference), and replace the placeholder .server-changes/2026-XX-XX filename with the real dated file. Addresses CodeRabbit notes on the design docs.
A prod-like A/B benchmark for the concurrency-key virtual-time scheduling change. Includes a method doc (hypotheses, scenarios, metrics, results template), a queue-level micro-benchmark that drives RunQueue with the flag off vs on under identical load (reuses the fairness test harness; inert in CI unless CK_BENCH_REDIS_URL is set), and a deployable end-to-end noisy-neighbor trigger project. All numbers are relative (same box, same load) so the scheduler is isolated from absolute throughput.
Enumerate runs from a trigger-time id manifest instead of runs.list (the list API can return empty on self-hosted, where it is ClickHouse-backed), add waitdrain and preflight helpers, pin the CLI and SDK to a matching version, and document the deploy invocation that actually works on self-hosted: no --self-hosted or --local-build, and --network host so the in-build indexer step can reach the instance API.
Real flag OFF-vs-ON A/B from a local homelab with a production-like multi-cluster topology (not production, no prod data or traffic). Micro arm: a light key behind a backlog goes from first-served at step 480 to step 4, wait p95 down ~70%, contention fairness 0.34 to 1.0, with the balanced and lone-key cases unchanged and drain steps identical (work-conserving). End-to-end arm across three worker clusters: the victim tenant start latency drops ~38% mean while the flood tenant is unchanged. Relative numbers only; single box, not prod scale.
Ignore the generated micro-benchmark output (bench-results/) and the standalone e2e harness run artifacts (node_modules, e2e-results, manifests, .env, lockfiles) so benchmark runs do not dirty the tree.
…ardinality Adds a resource/cardinality arm to the CK virtual-time benchmark: a plan, a runnable harness (drives a real RunQueue against a dedicated Redis, flag OFF vs ON, and reads server-side INFO memory/cpu, MEMORY USAGE and OBJECT ENCODING; inert without CK_BENCH_REDIS_URL), and results from a local homelab. Findings: the :ckVtime ZSET is essentially a second copy of :ckIndex (~150 bytes per key), so memory is linear in cardinality (about +1.3MB for a 10k-key queue) and TTL-reclaimed; Redis CPU overhead is small and does not scale with cardinality (+5 to +12 percent on an identical workload, per-script cost flat at ~25 usec/call) because the dequeue window is fixed and the ZSET ops are O(log N); and ckVtime membership tracks ckIndex exactly under sustained churn.
Keeps the change to the scheduler code, the CI tests, and the server-changes note. The benchmark harnesses, e2e project, results, plans, and design references were only ever local validation aids and don't belong in the repo.
…me floor A concurrency-key variant that has queued work but nothing ready yet, which is what every nack with a retry backoff produces, stayed registered in ckVtime holding its old low tag. The floor is the lowest stored tag, so it froze there while the keys actually being served advanced. New keys register at the floor, so a key that arrived later started well below the established ones and won every pass-1 slot until it caught up, which is the starvation the feature is meant to remove. The dequeue path now de-registers a variant when it has work but none of it is ready, alongside the existing GC for variants with no work at all. It stays in ckIndex, so pass 2 still serves it in age order once its head is ready, and it rejoins the fair order at the current floor on its next enqueue, nack or serve. Idle keys no longer hoard priority credit either. Measured with a control against a treatment on the real dequeue path: with one future-headed variant present the floor stayed at 0 while served keys reached 25, and a newcomer took 20 of the next 20 serves. With the fix the same run matches the control, newcomer 5 of 20. Reported by Devin on #4367.
…ants The floor only tracked the lowest tag on record, so any registered variant that could not be served held it there: one sitting at its own concurrency ceiling, or one whose head is not ready yet. The keys actually being served advanced past it, and since new keys register at the floor, a later arrival started underneath the incumbents and took the fair pass until it caught up. The floor now also rises to the lowest tag that was servable on the call. Pass 1 walks candidates in ascending tag order, so that is a safe lower bound. The repair from the lowest tag on record stays, because both routes only ever raise it and it still recovers a floor that was lost while ckVtime survived. Also adds the fairness scenario the suite was missing. None of the six scenarios nacked or used future-scored messages, so a stalled variant never existed and this class of bug could not show up. In the window after it lands the latecomer now takes 5 of 20 serves, against 12 of 20 without the fix.
Reverts the de-registration added earlier on this branch. Dropping a variant from ckVtime when it had work but nothing ready stranded it: pass 1 is the only reader of ckVtime, and pass 2 is skipped whenever pass 1 fills the batch, so on a queue busy enough to keep filling it the variant was never looked at again. A blind review measured one sitting unserved for over two thousand calls after its head became ready, and every nack backoff produces exactly that shape, so a single steady key could hold up another key's retries indefinitely. That is worse than the floor pinning it was meant to address. The floor advance from servable variants handles both cases on its own, so the de-registration bought nothing. It now also only takes its bound from pass 1. Pass 2 picks candidates by message age, so its tag implies nothing about the entries it skipped, and letting it move the floor stepped over registered variants that were servable and simply never visited, confiscating their credit on the next serve. Adds the regression test the earlier tests were missing. They proved the variant was evicted but never that it came back, which is the half that was broken.
At the moment the flag is flipped, :ckVtime is empty and every already-queued variant is unregistered. The first dequeue has an empty pass 1, so pass 2 serves and registers up to actualMaxCount variants. From the next call on, pass 1 can serve one message per registered variant and actualMaxCount is often small, so pass 1 fills the batch off that cohort alone. Pass 2 was gated on dequeuedCount < actualMaxCount, so it never ran again, and the rest of the backlog stayed invisible until a registered variant fully drained or an enqueue or nack happened to land on it. A key that gets no further work has no other route into the fair order. Same reachability shape 1a6d1a5 fixed for registered-but-unservable variants, applied to the unregistered cohort. It also covers a mixed deploy, where an instance with the flag still off enqueues through the non-vtime command, and a :ckVtime that expired while ckIndex lived. Pass 2 now always runs. When the batch is already full it registers the variants pass 1 could not see, at the floor, instead of serving them, so the next call's pass 1 leads with them. Serving is still capped at actualMaxCount, so no serve happens that the old gate would have refused, and the fairness scenarios are byte-identical: ckSkew, ckTrickle, ckSybil, ckBalanced, ckManyKeys, ckHeavyIdle and ckStalledNewcomer all report the same numbers as before. Op cost is one extra fixed op per call. The pass-1 window read doubles as a free membership set, so nothing is registered twice, and the registrations are collected into a single variadic ZADD NX rather than one call each. Measured on the op-count budget test: 11.62 ops per dequeue over the flag-off path, against 10.90 before. The budget comment now counts 8 fixed ops rather than 7. Devin's suggestion of reserving a batch slot for pass 2 does not fix it. Pass 2 walks ckIndex in age order, so its one reserved slot always lands on a variant that is already registered and never reaches the cohort that is not. Measured against the new tests: identical to no fix at all, 12, 16 and 12 calls. Reported by Devin on #4367.
The scenario said a bounded first-serve delay was fine without asserting any bound, so nothing stopped that delay growing. It now pins the measured values: the light key is first served on step 9 with the flag on against 72 with it off, and cardinality above the pass-1 window costs no throughput (drain 79 on, 81 off). The harness has no wall-clock wait and no randomness, so those figures are exact; the assertions carry a little slack for tie-break churn only.
… idle polls writing Pass 1 now steps over a variant whose head is scheduled in the future without spending one of its window slots, so a retry storm across enough keys can no longer fill the window with variants that cannot be served and freeze the virtual-time floor. The variant stays registered and stays scanned, which is what keeps it reachable; only the budget is spared. The scan is capped at twice the window, so a wider block still degrades to pass 2's age order. A dequeue that serves nothing now persists nothing. Both things that block would write are re-derivable: minServableTag is only set inside a successful serve, and discovery only runs once the batch is full, so the floor read-repair is recomputed from ckVtime on the next call anyway. Refits the two tests whose premise these change: the freeze test now pins the residual beyond the scan cap, and the floor test pins that a zero-serve call persists nothing while the repair still lands on the next serving call.
Carries the fix from #4628 into the three CK scripts this branch adds, which do not exist on main and so could not be covered there. A concurrency key of '*' renders a variant name identical to the wildcard member the master queue uses for the base queue, and the unguarded transition cleanup then removed the entry the rebalance had just written, stranding every concurrency key on that queue. The pre-existing scripts are fixed in #4628; this is the same one-line guard applied to enqueueMessageCkVtimeTracked, enqueueMessageWithTtlCkVtimeTracked and nackMessageCkVtimeTracked.
…r order tryServe marks a variant attempted before the per-key concurrency gate, and pass 2's discovery step skips anything attempted, so a variant that was both gated and unregistered fell through every route: pass 1 could not see it without a ckVtime entry, pass 2's attempt was a no-op behind the gate, and discovery then passed over it. It stayed invisible to the fair pass on every call for as long as the gate held, and only a serve would have registered it. Unregistered only arises where a variant reached ckIndex without a vtime-aware write, which is the rollout case discovery already exists to repair: a backlog queued before the flag went on, an enqueue from an instance that still has it off, or a ckVtime that expired while ckIndex lived. Registration is NX so an already-registered variant keeps the tag it earned, and the state TTL is only written when the ZADD actually registered something, which is the path that can recreate a ckVtime key that expired out from under a live ckIndex. Adds a regression test that fails without the branch, and a second op-count budget covering the all-unservable scan. The existing budget only bounds the servable shape (its fixture acks immediately so nothing is ever gated or deferred), and its comment read as a general worst case, which it is not. Reported by Devin on #4367.
… TTL expiry and dead-letter Only the vtime dequeue removed a variant from :ckVtime. Ack, TTL expiry and the dead-letter path all ZREM a drained variant from ckIndex and left its ckVtime entry behind, with a tag that had stopped advancing, until some later scan happened to visit and collect it. Ack is much the most common of the three: it is what a cancellation of a still-queued run runs through. The limitations note called this bounded and self-healing, and it is, but on two weaker grounds than it claimed. The 24h state TTL is refreshed by every enqueue, nack and serving dequeue, so on a queue that is never quiet for a full day it never fires, leaving floor advance as the only collection route. And that route can be held still by a workload that keeps minting concurrency keys, because each fresh variant registers at the floor and its first serve records the floor as the minimum servable tag. It is also not fairness-neutral: registration is NX, so a REUSED key inherits the stale high tag and is deprioritised, which means a cancel-drain remembers history that a serve-drain forgets. Adds vtime variants of the three commands rather than editing them, so the flag-off scripts stay byte-identical by construction (this is a pure addition to the file). Ack and dead-letter take the key as one more KEYS slot from the call site; the TTL sweep derives it in Lua, as it already derives ckIndexKey, because it discovers the queues it touches inside the script. Tests cover all three paths and fail without the fix. The old stranded-entry test is kept rather than deleted, retargeted at the pre-fix command directly, since GC-on-scan is still load-bearing for an older instance during a rolling deploy. Reported by Devin on #4367.
A concurrency-key variant whose message zset drained was removed from ckVtime, throwing away the virtual-time tag it had accumulated. Its next enqueue re-registered it at the floor, so it came back with full credit. A variant holding a persistent backlog keeps advancing its tag instead, so it lost every pass-1 slot to variants that drain and reset each call, and pass 2 could not help once the batch was already full. Measured on a backlogged variant against five trickle variants: 1 serve out of 600 with the flag on, against 120 with it off, inverting the fairness the feature exists to provide. A parked tag now survives the drain in a sibling :ckVtimeIdle zset, and every registration path (enqueue, enqueue-with-ttl, nack, and the gated-variant branch) starts the variant at max(floor, idleTag) rather than at the floor. The out-of-band drains (ack, dead-letter, TTL expiry) park the tag too. Entries at or below the floor confer nothing, so a single ZREMRANGEBYSCORE per serving call reaps them and bounds the set. Deriving the floor differently was tried first and rejected by measurement: any variant with a tag that never advances, which includes any concurrency-gated key, pins the minimum and defeats it. Backlogged variant now lands on its round-robin share in every shape, including with a pinned-low gated or future-headed variant present, and with more trickle variants than batch slots. Op-count overhead goes from 587 to 641 against a budget of 900. The flag-off Lua is still byte-identical: 7 vtime command variants changed, the other 31 commands hash the same as HEAD.
The at-or-below-floor reap on :ckVtimeIdle is worth nothing while the floor is pinned, and a workload that keeps minting fresh concurrency keys pins it indefinitely: each new key registers at the floor and is served at it, so minServableTag never rises. A resource benchmark caught the set growing by the drain count every round and never shrinking, passing ckIndex in size by round 50 and reaching 12000 entries (1.77MB) over 60 rounds, with only the 24h state TTL bounding it. The mechanism was measured rather than inferred: a probe sampling the floor found it at 0 on every round while the lowest parked tag was 1, so ZREMRANGEBYSCORE could never match. Adds a rank cap, keeping the highest idleMaxEntries tags (default 10000, configurable), which does not depend on the floor moving. Trimming the lowest tags first drops the entries nearest the floor, whose remembered credit is worth least. Verified against an explicit cap of 3000: the set rises to it and stays flat there across 12000 drains with the floor still pinned at 0. The new ARGV is inserted before the metrics gauge arg, which has to stay last because the gauge fragment reads ARGV[#ARGV]. Also drops the node:test describe import from the new test file, which shadows vitest's own under globals:true. That is a wider pattern in this directory and is left alone elsewhere.
9b5d58a to
1bfab30
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…path The idle lookup only matters on the call that actually registers a variant. ZADD NX is a no-op on one that is already registered, and its tag is already correct, so the ZSCORE preceding it was wasted on every enqueue after the first. Doing the ZADD first and the ZSCORE only when it reports an insert takes the common path from two ops to one. The per-registration EXPIRE of ckVtimeIdle went too: the park sites are the only writers that put anything in that key and they set its TTL themselves, so refreshing it on a call that may never write there was pure cost. Applies to all four registration sites: enqueue, enqueue-with-ttl, nack, and the gated-variant branch in the vtime dequeue. Six redis.call per registration becomes four in the common case, five or six on the rarer call that registers. Measured on a saturated benchmark (generator co-located with Redis, 1M invocations per arm, 3 interleaved cycles, Redis CPU/wall 0.97 on every arm, two independent cost measures agreeing to 0.04 usec). Against the flag-off enqueue script at 8.603 usec, the vtime path was 10.903 usec (+26.7%) and is now 10.177 usec (+18.3%), so this removes about 30% of the virtual-time enqueue overhead. That +26.7% independently reproduces the 26/23/24% total-CPU overhead the cardinality benchmark measured by a different method. A probe isolating the block gives the model behind it: roughly 1.38 usec fixed per EVALSHA plus 0.33 usec per redis.call, linear in call count for O(1) commands on small keys. Behaviour is unchanged. Final ckVtime tags are identical across already registered, unregistered, idle above floor, idle below floor and missing floor key, and the starvation suite that asserts exact tags still passes.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
… order A concurrency-gated candidate cost two Redis calls: the SCARD that discovers it is gated, and a ZADD NX to make sure it is in the fair order. For anything pass 1 selected the second is a guaranteed no-op, because pass 1 draws its candidates from the ckVtime zset and being in that scan is what registration means. tryServe now takes a knownRegistered flag, so a gated visit from pass 1 costs only the SCARD, the same as the flag-off command. Pass 2 candidates can genuinely be unregistered, and ones outside pass 1's scanned prefix are indistinguishable from those, so both are collected and settled after pass 2 by a single variadic ZADD NX. Its return value is the count it inserted, so the idle-tag correction that stops a drained variant reclaiming full credit only runs when something actually registered. In the steady state nothing does and the whole batch costs that one call. Measured on the fully-gated shape, saturated (generator co-located with Redis, 1M invocations per arm, Redis CPU/wall 0.978-0.987, no state drift): N=1000 flag-off 21.05 before 76.63 (+55.6) after 54.56 (+33.5) -22.1us, 40% N=10000 flag-off 21.09 before 77.99 (+56.9) after 57.52 (+36.4) -20.5us, 36% A batched ZMSCORE reads the same information and measured better, -24.9us and -24.7us for 45% and 43%. It was rejected anyway: it would have been the first thing in this file to require Redis 6.2, and about 3 to 4 usec is a fair price for not raising the floor. The variadic ZADD NX is one call either way; it costs more because thirty skiplist lookups on the write path are dearer than thirty reads. Worth noting the saving per call removed is nearer 0.4 usec than the 0.33 usec measured previously on small keys, because the call being removed is a write against a zset holding thousands of members. Fully gated is the worst case by construction: a dequeue that serves exits earlier. Per-key concurrency limits are ordinary on ck queues, so it is worth having.
df44847 to
845c792
Compare
|
@ericallam re-ran these, since a review turned up a fairness bug and the fix for it added a third Redis key plus some work on the hot path. Memory's fine. Dequeue CPU was the ugly one and it's about a third better now.
That "overhead shrinks as the queue grows" line from last time was wrong, ignore it: enqueue is flat and the gated scan rises then plateaus. I'd taken it off a rig that only got redis to about 5% utilisation, which turns out not to resolve differences this size at all (two runs of it on near-identical code disagreed by more than the effect I was trying to measure, which is what tipped me off). These are from a saturated setup instead, generator sat next to redis, one script at a time by sha, 1M invocations an arm, CPU/wall 0.98 throughout. Useful rule of thumb that fell out: roughly 1.38 usec fixed per EVALSHA plus 0.33 per Bigger swing at the gated scan is parked on |
…scores The gated-registration path in the vtime dequeue was the one writer that touched :ckVtime without also touching :ckVtimeFloor. Everything else keeps the pair alive together: the enqueue and nack registrations EXPIRE both, and a serving dequeue SETs the floor with its own TTL. That path runs on calls that serve nothing, so the floor-persist block, which is guarded on having served, is skipped. A base queue whose variants are all sat at their per-key ceiling therefore refreshes the tags on every poll while the floor's TTL runs down underneath them. Once it expires the next registration reads GET ckVtimeFloorKey back as '0' and starts a brand-new variant below every established tag, so it leads pass 1 until it catches up. Same hole as the one the enqueue floor-TTL test was added to close, reached by a different path. Reported by Devin on #4367.
… change The note ran to three sentences, advertised a server env var a user cannot set, and described the pass-1 window degradation. .server-changes/README.md asks for a one-line description of behaviour rather than implementation, and says that needing a paragraph usually means you are describing the implementation. Reported by Devin on #4367; this is its suggested wording.
| if redis.call('ZADD', unpack(gatedArgs)) > 0 then | ||
| for _, ckQueueName in ipairs(gatedPending) do | ||
| local gateIdle = redis.call('ZSCORE', ckVtimeIdleKey, ckQueueName) | ||
| if gateIdle and tonumber(gateIdle) > floor then | ||
| redis.call('ZADD', ckVtimeKey, 'XX', gateIdle, ckQueueName) | ||
| end | ||
| end |
There was a problem hiding this comment.
🟡 Fair-share credit can be handed back to a concurrency-key that already used it, letting it jump the queue
When a batch of concurrency keys is checked for registration, the leftover-credit correction is applied to every key in that batch (ZADD ... 'XX' at internal-packages/run-engine/src/run-queue/index.ts:5620) instead of only the ones just registered, so a key that already spent its turn can have its clock wound backwards and get served ahead of keys that are genuinely due.
Impact: On a task queue with many concurrency keys, one key can repeatedly take more than its fair share of runs, delaying the others the feature is meant to protect.
Mechanism: the XX correction loop iterates all of `gatedPending`, not just the entries the NX add created
gatedPending collects candidates whose knownRegistered flag was false. knownRegistered comes from the registered set, which is built only from ZRANGE ckVtimeKey 0 scanLimit-1 (internal-packages/run-engine/src/run-queue/index.ts:5549-5557). When a base queue has more CK variants than scanLimit (= 2 * maxCount * windowMultiplier, i.e. 60 with the defaults), that set is truncated, so an already-registered variant that is concurrency-gated in pass 2 is pushed into gatedPending (internal-packages/run-engine/src/run-queue/index.ts:5541-5544).
The batch ZADD ... NX at internal-packages/run-engine/src/run-queue/index.ts:5616 correctly no-ops for it. But the guard is only > 0 ("something in the batch was added"), and the correction loop then runs for all members of gatedPending. For the already-registered variant, ZSCORE ckVtimeIdle can still hold a stale parked tag above the floor (the idle entry is never deleted when a variant re-registers via the enqueue path at internal-packages/run-engine/src/run-queue/index.ts:4363-4369, and is only reaped once the floor passes it at internal-packages/run-engine/src/run-queue/index.ts:5648). The ZADD ... 'XX' then overwrites its current, advanced tag with that older idle tag — a rewind of the virtual clock, which is exactly the advance-only invariant the rest of the script maintains (NX on every registration path).
The fix is to restrict the correction to the members the NX add actually created, e.g. by checking membership per candidate before the NX add, or by only correcting candidates whose individual ZADD NX returned 1.
Prompt for agents
In `dequeueMessagesFromCkQueueVtimeTracked` (internal-packages/run-engine/src/run-queue/index.ts), the gated-candidate registration block issues one variadic `ZADD ckVtimeKey NX <floor> <name>...` for every entry in `gatedPending`, and then — if the ZADD added at least one member — loops over ALL of `gatedPending` applying `ZADD ckVtimeKey XX <idleTag> <name>` whenever `ZSCORE ckVtimeIdle` is above the floor.
The problem: `gatedPending` can contain variants that are already registered with an advanced tag. That happens because `knownRegistered` is derived from the `registered` set, which is built from a truncated `ZRANGE ckVtimeKey 0 scanLimit-1`; when a base queue has more CK variants than `scanLimit` (2 * maxCount * windowMultiplier, 60 by default) the set is incomplete. Stale `ckVtimeIdle` entries also survive re-registration (the enqueue path reads the idle tag but never deletes it; it is only reaped once the floor passes it). So the `XX` write can overwrite a live, advanced tag with an older parked tag, rewinding that variant's virtual clock and giving it unearned priority in pass 1 — the opposite of the fairness guarantee.
The idle-tag correction must apply only to variants this call actually registered. Options: perform the NX add per candidate and only correct when its own return value is 1, or capture which members were absent before the batched NX (per-candidate ZSCORE before the add), or gate the correction on `not registered[name] and <not already a member>`. Keep the op-count budget asserted in ckVtimeConcurrency.test.ts in mind — the steady state should still cost a single ZADD.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Off by default.
When many concurrency-key variants share one task queue, the dequeue serves the oldest waiting run first, so one key's large backlog is served to exhaustion while keys queued behind it wait for the whole pile to drain. This adds an opt-in fair order: each key gets a virtual clock, the dequeue serves the smallest clock and advances it, so keys take turns instead of one pile draining. With the flag off, the existing scripts run unchanged.
How it works
:ckVtimeZSET (the virtual clocks) and a monotonic floor.ckIndexkeeps its head-timestamp domain, so time-eligibility, master-queue rebalancing, and every other writer stay untouched (this is what makes it mixed-deploy safe).Testing
Full run-queue suite is green with the flag off (no regression). Fairness is proven on the real batched dequeue path (not just one message per call), plus multi-consumer exactly-once, a per-dequeue op-count budget, and behaviour tests for the floor, tag advance, GC, and registration.
Rollout
Off by default behind
RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED. Enable on a staging cell, then production; rollback is flipping the flag off (leftover state expires within a day). During a rolling deploy, old instances serve in age order and are folded in by pass 2, so nothing is lost and no run is served twice. Every mutation is a single atomic Lua script, which is what makes those interleavings safe. That atomicity assumes the single-node Redis the run queue actually runs on: it has no cluster-mode setting (every other Redis inenv.server.tshas one,RUN_ENGINE_RUN_QUEUE_REDIS_*doesn't), and the master queue key sits outside the base queue's hash slot exactly as it does in the command this one is modelled on.Known limitations
A few review-flagged edges are bounded and self-heal rather than block the flag: variants drained by ack, TTL expiry, or the dead-letter path aren't removed from the virtual-time set, so a stale low-tag entry heals itself on the very next dequeue while a stale high-tag entry is just inert memory that clears within the 24h state TTL. Ties between variants sitting at the same virtual-time tag (a cold start, or a garbage-collected variant re-registering) break by queue-name lexical order rather than anything meaningful, a pre-existing effect of the old head-timestamp ordering that only affects who is served first, not long-run fairness.
The third edge is the pass-1 window itself. Enqueue and nack register a variant in the virtual-time set whether or not its head message is ready, so variants waiting on a retry backoff or a future start time still take up window slots. If enough of them do that at once to fill the window (
maxCount * RUN_ENGINE_CK_VTIME_WINDOW_MULTIPLIER, 3x by default), the fair pass serves nothing, the floor stops advancing, and every serve comes from pass 2 in today's age order. Work conservation still holds, so this is fairness degradation under a retry storm rather than a stall, and a key that arrives during one registers at the frozen floor and then leads by the virtual time the incumbents accrued while it lasted. Widen the multiplier if it shows up.ckVtime.test.tspins both halves: the degraded pass still serves on every call, and the recovery is bounded.