diff --git a/.server-changes/2026-07-24-ck-fair-scheduling.md b/.server-changes/2026-07-24-ck-fair-scheduling.md new file mode 100644 index 00000000000..d9e04336819 --- /dev/null +++ b/.server-changes/2026-07-24-ck-fair-scheduling.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: feature +--- + +The run queue can now serve concurrency keys fairly, so one key with a large backlog no longer starves runs waiting on other keys. It is opt-in and off by default. diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 53b5edf0a46..6b583e29c11 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -941,6 +941,13 @@ const EnvironmentSchema = z RUN_ENGINE_TTL_CONSUMERS_DISABLED: BoolEnv.default(false), RUN_ENGINE_TTL_WORKER_BATCH_MAX_WAIT_MS: z.coerce.number().int().default(5_000), + // Fair (virtual-time) ordering across concurrency-key variants of a base queue. + // Off by default; when off the run queue behaves exactly as before. + RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED: BoolEnv.default(false), + RUN_ENGINE_CK_VTIME_QUANTUM: z.coerce.number().int().positive().default(1), + RUN_ENGINE_CK_VTIME_WINDOW_MULTIPLIER: z.coerce.number().int().positive().default(3), + RUN_ENGINE_CK_VTIME_STATE_TTL_SECONDS: z.coerce.number().int().positive().default(86400), + /** Optional maximum TTL for all runs (e.g. "14d"). If set, runs without an explicit TTL * will use this as their TTL, and runs with a TTL larger than this will be clamped. */ RUN_ENGINE_DEFAULT_MAX_TTL: z.string().optional(), diff --git a/apps/webapp/app/v3/runEngine.server.ts b/apps/webapp/app/v3/runEngine.server.ts index 4d9e263d6be..9c47f10451c 100644 --- a/apps/webapp/app/v3/runEngine.server.ts +++ b/apps/webapp/app/v3/runEngine.server.ts @@ -106,6 +106,14 @@ function createRunEngine() { batchMaxSize: env.RUN_ENGINE_TTL_WORKER_BATCH_MAX_SIZE, batchMaxWaitMs: env.RUN_ENGINE_TTL_WORKER_BATCH_MAX_WAIT_MS, }, + ckVirtualTimeScheduling: env.RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED + ? { + enabled: true, + quantum: env.RUN_ENGINE_CK_VTIME_QUANTUM, + scanWindowMultiplier: env.RUN_ENGINE_CK_VTIME_WINDOW_MULTIPLIER, + stateTtlSeconds: env.RUN_ENGINE_CK_VTIME_STATE_TTL_SECONDS, + } + : undefined, }, runLock: { redis: { diff --git a/internal-packages/run-engine/design/plans/2026-07-23-ck-virtual-time-scheduling-plan.md b/internal-packages/run-engine/design/plans/2026-07-23-ck-virtual-time-scheduling-plan.md new file mode 100644 index 00000000000..26a8311d45e --- /dev/null +++ b/internal-packages/run-engine/design/plans/2026-07-23-ck-virtual-time-scheduling-plan.md @@ -0,0 +1,829 @@ +# Virtual-time (SFQ) scheduling for the concurrency-key dequeue + +Implementation and testing plan for the recommendation out of three fairness +spikes. The spike harness and benchmark code are archived on the remote branch +`chore/fair-queueing-spike` (throwaway, never merged); the findings and research +they produced are kept alongside this plan as references: +`internal-packages/run-engine/design/references/run-queue-fairness-ck-findings.md`, +`.../run-queue-fairness-caps-vs-scheduling-findings.md`, and +`.../run-queue-fairness-research.md`. + +The recommendation: score the per-base-queue concurrency-key selection by +start-time fair queueing (SFQ) virtual time instead of head timestamp, inside the +real batched CK-dequeue Lua, layered UNDER the existing per-key concurrency gate +and the planned group caps. Caps bound occupancy; the fair order bounds wait under +contention (the Kubernetes-APF shape, and the Parekh-Gallager joint result). + +## Goal + +When many concurrency-key variants of one base queue are contending, the +dequeue order across variants follows SFQ virtual time (each variant advances +its own virtual clock by a quantum per serve; new variants join at a monotonic +floor). This fixes the #2617 starvation dynamic the spikes measured: a key +arriving behind a big backlog waits its fair turn instead of waiting for the +backlog to drain, and (unlike per-key caps) the fix survives a tenant sharding +its work across many keys, while staying work-conserving. + +Everything is behind a constructor feature flag. Flag off is byte-identical to +today: the exact same Lua scripts run and no new Redis keys are ever touched. + +## Architecture + +The design keeps `ckIndex` exactly as it is and adds a parallel virtual-time +ZSET. This is the load-bearing decision, so the reasoning up front: + +`ckIndex` scores are head-message timestamps, and three things depend on that +score domain staying timestamps: + +1. Time eligibility. `ZRANGEBYSCORE ckIndexKey -inf now` filters out variants + whose head message is scheduled in the future (delayed runs, nack backoff). + Virtual-time tags carry no wall-clock meaning, so they cannot express "not + available yet". +2. Master-queue rebalancing. Every CK Lua (enqueue, dequeue, ack, nack, the + sweeper) re-scores the `:ck:*` master-queue member from `ZRANGE ckIndexKey + 0 0 WITHSCORES`. The master queue is timestamp-ordered and compared against + `now`; writing virtual times there would corrupt the shard-level selection. +3. Every other writer. `enqueueMessageCkTracked`, `nackMessageCkTracked`, + `acknowledgeMessageCkTracked`, the concurrency sweeper (index.ts ~3733, + ~3847) all `ZADD ckIndexKey `. Rescoring only in the dequeue + Lua would leave a mixed score domain, and during a rolling deploy old + instances would keep writing timestamps regardless (the mixed-arity hazard + the caps plan warned about, in score-domain form). + +So: instead of changing `ckIndex`'s score domain, add per base queue + +- `{org:...}:...:queue::ckVtime`, a ZSET, member = the full CK-variant + queue name (the same member strings `ckIndex` holds), score = the variant's + next virtual start tag (the spike's `SfqCk.clock` value, i.e. start of last + serve + quantum). +- `{org:...}:...:queue::ckVtimeFloor`, a STRING holding the monotonic + floor (the CFS `min_vruntime` analogue from `disciplines.ts`). + +Both live under the same `{org:...}` hash tag as every other key of the base +queue, so cluster slotting is unchanged and one Lua script can touch all of +them atomically. + +The dequeue Lua (new command, flag-selected) runs two passes: + +- Pass 1 (fair order): take candidates from `ckVtime` by rank (lowest tag + first, `ZRANGE 0 W-1 WITHSCORES`), and for each run the existing + per-candidate logic unchanged: per-key concurrency gate, per-variant + time-eligibility check (`ZRANGEBYSCORE -inf now LIMIT 0 1`), TTL + branch, counters, `ckIndex` rebalance. On each successful serve, advance + that variant's tag (`ZADD ckVtimeKey max(tag, floor) + quantum/weight`) + before moving to the next candidate, so the state is correct per serve + within the batch. A skipped variant (at cap, or head in the future) keeps + its tag: no service, no advance, which is the SFQ rule. +- Pass 2 (fill + discovery): if pass 1 served fewer than `actualMaxCount`, + scan `ckIndex` in today's age order (`ZRANGEBYSCORE -inf now LIMIT 0 W`), + skip variants already attempted in pass 1, and serve the rest through the + same per-candidate logic, registering each served variant into `ckVtime`. + Pass 2 makes the new command a strict superset of today's: it can never + serve fewer messages than the current script would, so work conservation + and mixed-deploy discovery both hold by construction. + +Registration (how a variant gets INTO `ckVtime` before it is ever served): +every Lua that adds messages to a variant, i.e. the CK enqueue commands and +the CK nack command, gains a flag-selected variant that does +`ZADD ckVtimeKey NX ` after its existing `ckIndex` rebalance. +`NX` means registration can never rewind an advanced tag. This is what makes +the sybil case work: a brand-new light key is present in the vtime order at +the floor from its first enqueue, so it is reachable in pass 1 even when a +hundred attacker variants have older heads (which is exactly where today's +age-ordered `*3` window fails, per CAPS_FINDINGS). + +Closure argument for registration (state this as an invariant and test it): +a variant's queue becomes non-empty only via enqueue or nack, both of which +register. The sweeper and ack/release Luas only rebalance variants whose +queues are already non-empty, so they never need to register. The dequeue Lua +GCs a variant from BOTH `ckIndex` and `ckVtime` when its queue is empty, so +membership stays closed under all transitions. The one gap is old-code +enqueues during a rolling deploy, and pass 2 covers that (served via age +order, registered on serve). + +Batched-call semantics: the current Lua serves at most ONE message per variant +per call (`LIMIT 0, 1` per candidate, and ZSET members are unique in the +candidate list). The new command keeps that. Within one call the batch is +therefore one-serve-per-variant round robin over the `actualMaxCount` lowest +tags, and each serve's `ZADD` makes the NEXT call's order correct. This +deviates from pure SFQ within a single batch (pure SFQ could serve the same +far-behind variant several times in a row) but converges across calls, and +one-per-variant is itself a fair schedule. Preserving it also means zero +change to today's per-call throughput shape. + +Layering with caps: the per-key gate (`ckCurrentConcurrency < +queueConcurrencyLimit`) stays exactly where it is, ahead of the serve. The +planned Phase-1 `:groupConcurrency`/`:totalConcurrency` total cap and Phase-2 +`:ckLimits` per-key overrides slot into the same per-candidate position as +additional admission conditions when they land; virtual time only decides the +ORDER among candidates those gates admit. Nothing in this plan blocks or is +blocked by the caps work, and the fairQueue-level "queue at total" drop stays +untouched (the CK pick is below the `RunQueueSelectionStrategy` interface; +`fairQueueSelectionStrategy.ts` is not modified). + +Weights: concurrency keys carry no configured weight today, so every key gets +weight 1 (quantum advance of 1.0 per serve). The advance is written as +`quantum / weight` with `weight` a named local fixed at 1, so a future +per-key weight (e.g. a sparse `:ckWeights` HASH mirroring the Phase-2 +`:ckLimits` shape) is a one-line change at the marked site. Justification for +equal-weight first: the spikes only measured equal weights, no product surface +exists to set a weight, and SFQ's starvation fix does not depend on weights. + +State lifecycle (GC/TTL), since concurrency keys are client-chosen and +unbounded: + +- Per-variant GC: whenever the dequeue Lua finds a variant queue empty it + already `ZREM`s the variant from `ckIndex`; the new command also `ZREM`s it + from `ckVtime` at those sites. A GC'd key that returns re-registers at the + floor, which is standard SFQ flow re-entry (history is forgiven when a flow + drains; a drained flow was by definition not backlogged). +- Whole-key TTL: `ckVtime` and `ckVtimeFloor` get `EXPIRE ` + (default 86400, matching the `counterTtlSeconds` precedent) refreshed on + every write. An idle base queue's vtime state evaporates; on resumption + everyone re-enters at floor 0, which is a clean restart. If only the floor + key expires, tags in `ckVtime` still self-heal because every read applies + `max(tag, floor)` and the next dequeue re-advances the floor to the minimum + stored tag. +- Cardinality guard: tags are only created for variants that actually have + queued messages (registration happens on enqueue/nack, GC on empty), so + `ckVtime` cardinality is bounded by `ckIndex` cardinality plus transiently + stale entries awaiting scan-time GC or TTL expiry. + +Floor semantics: on each dequeue call, `floor = max(stored floor, score of +ckVtime rank 0)`, written back with the TTL. The floor never decreases (test +this), and a newly registered key's tag starts AT the floor, so it can never +be scheduled behind the accumulated backlog of long-running keys (the SFQ +property; this is the exact `SfqCk` logic from `disciplines.ts`, moved into +Lua with the Map replaced by the ZSET and the floor by the STRING). + +Numeric domain: tags are Redis doubles starting at 0 advancing by 1.0 per +serve; integer-exact to 2^53 serves per base queue, so precision is a +non-issue. + +The scan window: pass 1's window is `actualMaxCount * windowMultiplier` +(default multiplier 3, same as today, made configurable). The score domain of +the window changes meaning: today the window can hide the oldest ELIGIBLE +head behind at-cap variants with older heads; under vtime it can hide the +lowest ELIGIBLE tag behind at-cap or future-scheduled variants with lower +tags. Those clogging variants keep low tags while skipped (no serve, no +advance), so the failure shape is symmetric with today's, and it is the same +class of limitation CAPS_FINDINGS documents for the `*3` window rather than a +new one. We do not widen the default; we make the multiplier an option so an +operator can widen it if per-key caps plus heavy nack backoff ever clog a +window in practice, and pass 2 guarantees the call still finds work. + +## Tech stack + +- Redis Lua (ioredis `defineCommand`) in + `internal-packages/run-engine/src/run-queue/index.ts`, following the + existing tracked-command patterns. +- TypeScript for options plumbing and call-site selection. +- vitest + `@internal/testcontainers` (`redisTest`) for all tests. No mocks. +- Verification: `pnpm run typecheck --filter @internal/run-engine` and + `cd internal-packages/run-engine && pnpm run test --run`. + +## Global constraints + +- Flag off must be byte-identical: off-path call sites keep calling the + existing command names whose script text is not edited at all. New + behaviour lives only in NEW command names (`...Vtime...`), selected in TS. + This is why we add command variants instead of threading an `enableVtime` + ARGV through existing scripts: an ARGV-gated single script would still be a + new script body (new SHA, new arity risks) even when the flag is off. +- `ckIndex`, the master queue, `fairQueueSelectionStrategy.ts`, and all + ack/release/sweeper Luas keep their current score domain and text. +- The dead untracked `dequeueMessagesFromCkQueue` (index.ts ~3999-4141) is not + touched and gets no vtime variant. +- All vtime state mutations happen inside single Lua scripts (atomic; Redis + serialises scripts, which is the whole multi-consumer correctness story). +- No process-memory scheduling state anywhere. +- Do not import anything from `fairness-spike-ck/` or `fairness-spike/` into + production code or the new tests; those directories are throwaway (their + own headers say delete before merge). Port logic and scenario shapes by + copying, with attribution comments. +- Zod stays at the repo-pinned version; no new dependencies. +- Formatting/lint before commit: `pnpm run format && pnpm run lint:fix`. + +## File structure + +Modify: + +- `internal-packages/run-engine/src/run-queue/keyProducer.ts` + (two new key builders + constants) +- `internal-packages/run-engine/src/run-queue/types.ts` + (`RunQueueKeyProducer` interface additions) +- `internal-packages/run-engine/src/run-queue/index.ts` + (options field; four new `defineCommand`s; TS module augmentation for them; + flag switches at the enqueue/nack/dequeue call sites; span attributes) +- `internal-packages/run-engine/src/run-queue/tests/keyProducer.test.ts` + (key builder tests) +- `internal-packages/run-engine/src/engine/types.ts` + (`RunEngineOptions["queue"].ckVirtualTimeScheduling`) +- `internal-packages/run-engine/src/engine/index.ts` + (pass the option through to `new RunQueue({...})`, ~line 196) +- `apps/webapp/app/env.server.ts` + (`RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED` and friends) +- `apps/webapp/app/v3/runEngine.server.ts` + (wire env vars into the engine options, ~line 61 `queue:` block) + +Create: + +- `internal-packages/run-engine/src/run-queue/tests/ckVtime.test.ts` + (Lua behaviour: ordering, floor, tag init, advance-within-batch, GC, TTL, + registration, pass-2 fill, flag-off keyspace purity) +- `internal-packages/run-engine/src/run-queue/tests/ckVtimeFairness.test.ts` + (ported scenarios at batched maxCount, wait/share/work-conservation/sybil + assertions) +- `internal-packages/run-engine/src/run-queue/tests/ckVtimeConcurrency.test.ts` + (multi-consumer correctness, op-count budget) +- `.server-changes/2026-07-24-ck-fair-scheduling.md` (at PR time; note it + ships dark) + +## Tasks + +### Task 1: key producer additions + +Files: `keyProducer.ts`, `types.ts`, `tests/keyProducer.test.ts`. + +Test first (append to `tests/keyProducer.test.ts`, matching its existing +style): + +```ts +it("produces ckVtime keys from a CK variant queue name", () => { + const keys = new RunQueueFullKeyProducer(); + const q = "{org:o1}:proj:p1:env:e1:queue:task/my-task:ck:tenant-a"; + expect(keys.ckVtimeKeyFromQueue(q)).toBe( + "{org:o1}:proj:p1:env:e1:queue:task/my-task:ckVtime" + ); + expect(keys.ckVtimeFloorKeyFromQueue(q)).toBe( + "{org:o1}:proj:p1:env:e1:queue:task/my-task:ckVtimeFloor" + ); + // ck wildcard and base-queue inputs normalise the same way + expect(keys.ckVtimeKeyFromQueue(q.replace(":ck:tenant-a", ":ck:*"))).toBe( + keys.ckVtimeKeyFromQueue(q) + ); +}); +``` + +Implementation in `keyProducer.ts`: add to `constants` + +```ts +CK_VTIME_PART: "ckVtime", +CK_VTIME_FLOOR_PART: "ckVtimeFloor", +``` + +and the builders (next to `ckIndexKeyFromQueue`): + +```ts +ckVtimeKeyFromQueue(queue: string): string { + return `${this.baseQueueKeyFromQueue(queue)}:${constants.CK_VTIME_PART}`; +} + +ckVtimeFloorKeyFromQueue(queue: string): string { + return `${this.baseQueueKeyFromQueue(queue)}:${constants.CK_VTIME_FLOOR_PART}`; +} +``` + +Add both signatures to `RunQueueKeyProducer` in `types.ts`. + +Verify: `cd internal-packages/run-engine && pnpm run test ./src/run-queue/tests/keyProducer.test.ts --run` +(new tests pass, existing pass), then +`pnpm run typecheck --filter @internal/run-engine` (clean). + +### Task 2: options plumbing in RunQueue + +File: `index.ts` (RunQueueOptions, ~line 60). + +```ts +/** + * Fair (virtual-time / SFQ) ordering across concurrency-key variants of a + * base queue. Off by default; when off, the exact pre-existing Lua commands + * run and no vtime keys are created. See internal-packages/run-engine/design/plans/ + * 2026-07-23-ck-virtual-time-scheduling-plan.md. + */ +ckVirtualTimeScheduling?: { + enabled: boolean; + /** Virtual-time advance per serve (dimensionless). Default 1. */ + quantum?: number; + /** Pass-1 candidate window = actualMaxCount * this. Default 3. */ + scanWindowMultiplier?: number; + /** EXPIRE applied to ckVtime/ckVtimeFloor on every write. Default 86400. */ + stateTtlSeconds?: number; +}; +``` + +Store resolved values once in the constructor (private readonly fields +`#ckVtimeEnabled`, `#ckVtimeQuantum`, `#ckVtimeWindowMultiplier`, +`#ckVtimeStateTtl`) so call sites read fields, never re-derive. + +Verify: `pnpm run typecheck --filter @internal/run-engine`. + +### Task 3: the vtime dequeue Lua (the core change) + +File: `index.ts`. New command `dequeueMessagesFromCkQueueVtimeTracked`, +`numberOfKeys: 12` (the 10 keys of `dequeueMessagesFromCkQueueTracked` plus +`ckVtimeKey`, `ckVtimeFloorKey`), plus its entry in the ioredis module +augmentation (next to the existing declaration at ~line 5591, same parameter +list plus `ckVtimeKey: string, ckVtimeFloorKey: string` after +`lengthCounterKey` and `quantum: string, windowMultiplier: string, +stateTtlSeconds: string` after `maxCount`). + +Write the failing tests FIRST in `tests/ckVtime.test.ts`. Scaffold the file +from `tests/ckIndex.test.ts` (same `testOptions`, `authenticatedEnvDev`, +`createQueue`, `makeMessage` helpers), with `createQueue` extended to accept +`ckVirtualTimeScheduling` overrides. Tests to write in this task: + +1. "vtime order beats head-timestamp order": enqueue 30 messages on + `ck: heavy` with timestamps `t0 .. t0+29`, then 3 messages on `ck: light` + at `t0+1000`. Register both (enqueue registration is Task 4; until then + the test seeds `ckVtime` directly with + `queue.redis.zadd(ckVtimeKey, 0, heavyVariant, 0, lightVariant)`). + Dequeue with `maxCount: 10` repeatedly (acking between calls to free + concurrency). Assert light's 3 messages are all served within the first 3 + calls (age order alone would drain heavy first). Assert each call returns + at most one message per variant. +2. "tags advance per serve within one batched call": seed 5 variants at tag + 0, one message each; one dequeue call with `maxCount: 5`; assert all 5 + served and `ZSCORE ckVtime ` is `1` for each (advanced inside the one + call, not once per call). +3. "floor is monotonic and read-repairs": drive tags to ~20 by repeated + serve of two keys, assert `GET ckVtimeFloor` never decreased across calls + (sample after each call), and equals the min stored tag after the last. +4. "new key initialises at the floor, not zero and not behind the backlog": + after tags reach ~20, register a fresh variant with the enqueue path (or + direct ZADD NX at the current floor pre-Task-4), enqueue one message on + it, one dequeue call; assert the fresh variant is served in that first + call and its tag afterwards is `floor + quantum`, not `1`. +5. "no service, no advance": set the base queue concurrency limit to 1 via + `queue.updateQueueConcurrencyLimits`, occupy `ck: a`'s slot (dequeue one, + do not ack), then call dequeue; assert `ck: a` was skipped, its tag is + unchanged, and other variants were served. +6. "GC on empty variant": drain a variant completely; assert it is removed + from BOTH `ckIndex` and `ckVtime`. +7. "TTL is set and refreshed": after any dequeue, `PTTL ckVtime` and + `PTTL ckVtimeFloor` are in `(0, stateTtlSeconds * 1000]`. +8. "pass 2 fill serves unregistered variants and registers them": enqueue on + a variant, delete its `ckVtime` entry by hand (simulating an old-code + enqueue), dequeue; assert the message is served AND the variant now has a + `ckVtime` tag. +9. "future-scheduled variants are skipped without advance": nack a message + with a future score (or enqueue with future timestamp); dequeue; assert + the variant is not served and its tag is unchanged. + +The Lua. Full sketch (the per-candidate serve body is today's tracked body +verbatim; only the parts marked NEW differ): + +```lua +local ckIndexKey = KEYS[1] +local queueConcurrencyLimitKey = KEYS[2] +local envConcurrencyLimitKey = KEYS[3] +local envConcurrencyLimitBurstFactorKey = KEYS[4] +local envCurrentConcurrencyKey = KEYS[5] +local messageKeyPrefix = KEYS[6] +local envQueueKey = KEYS[7] +local masterQueueKey = KEYS[8] +local ttlQueueKey = KEYS[9] +local lengthCounterKey = KEYS[10] +local ckVtimeKey = KEYS[11] -- NEW +local ckVtimeFloorKey = KEYS[12] -- NEW + +local ckWildcardName = ARGV[1] +local currentTime = tonumber(ARGV[2]) +local defaultEnvConcurrencyLimit = ARGV[3] +local defaultEnvConcurrencyBurstFactor = ARGV[4] +local keyPrefix = ARGV[5] +local maxCount = tonumber(ARGV[6] or '1') +local quantum = tonumber(ARGV[7] or '1') -- NEW +local windowMultiplier = tonumber(ARGV[8] or '3') -- NEW +local stateTtl = tonumber(ARGV[9] or '86400') -- NEW + +local function decrLengthCounter() + if tonumber(redis.call('GET', lengthCounterKey) or '0') > 0 then + redis.call('DECR', lengthCounterKey) + end +end + +-- env gate: identical to dequeueMessagesFromCkQueueTracked +local envCurrentConcurrency = tonumber(redis.call('SCARD', envCurrentConcurrencyKey) or '0') +local envConcurrencyLimit = tonumber(redis.call('GET', envConcurrencyLimitKey) or defaultEnvConcurrencyLimit) +local envConcurrencyLimitBurstFactor = tonumber(redis.call('GET', envConcurrencyLimitBurstFactorKey) or defaultEnvConcurrencyBurstFactor) +local envConcurrencyLimitWithBurstFactor = math.floor(envConcurrencyLimit * envConcurrencyLimitBurstFactor) +if envCurrentConcurrency >= envConcurrencyLimitWithBurstFactor then + return nil +end +local queueConcurrencyLimit = math.min(tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'), envConcurrencyLimit) +local envAvailableCapacity = envConcurrencyLimitWithBurstFactor - envCurrentConcurrency +local actualMaxCount = math.min(maxCount, envAvailableCapacity) +if actualMaxCount <= 0 then + return nil +end + +local window = actualMaxCount * windowMultiplier + +-- NEW: monotonic floor, advanced to the minimum stored tag +local floor = tonumber(redis.call('GET', ckVtimeFloorKey) or '0') +local minEntry = redis.call('ZRANGE', ckVtimeKey, 0, 0, 'WITHSCORES') +if #minEntry > 0 then + local minTag = tonumber(minEntry[2]) + if minTag > floor then + floor = minTag + end +end + +local results = {} +local dequeuedCount = 0 +local attempted = {} + +-- Per-candidate serve. Body between BEGIN/END COPY is today's tracked +-- per-candidate block, unmodified except the two NEW lines. +local function tryServe(ckQueueName) + attempted[ckQueueName] = true + local fullQueueKey = keyPrefix .. ckQueueName + local ckConcurrencyKey = fullQueueKey .. ':currentConcurrency' + local ckCurrentConcurrency = tonumber(redis.call('SCARD', ckConcurrencyKey) or '0') + if ckCurrentConcurrency >= queueConcurrencyLimit then + return + end + -- BEGIN COPY (from dequeueMessagesFromCkQueueTracked, lines ~4219-4273) + local messages = redis.call('ZRANGEBYSCORE', fullQueueKey, '-inf', tostring(currentTime), 'WITHSCORES', 'LIMIT', 0, 1) + if #messages >= 2 then + -- ... TTL-expired / normal-dequeue / stale-orphan branches verbatim ... + -- in the normal-dequeue branch, after dequeuedCount = dequeuedCount + 1: + -- NEW: advance this variant's virtual time (weight hook: fixed 1 today) + -- local weight = 1 + -- local tag = tonumber(redis.call('ZSCORE', ckVtimeKey, ckQueueName) or floor) + -- if tag < floor then tag = floor end + -- redis.call('ZADD', ckVtimeKey, tag + (quantum / weight), ckQueueName) + -- rebalance ckIndex from the variant head, verbatim, plus: + -- NEW: if the variant queue is empty, also redis.call('ZREM', ckVtimeKey, ckQueueName) + else + -- empty-in-range branch verbatim, plus the same NEW ZREM when fully empty + end + -- END COPY +end + +-- Pass 1: fair order (lowest virtual start tag first) +local vtimeCandidates = redis.call('ZRANGE', ckVtimeKey, 0, window - 1) +for _, ckQueueName in ipairs(vtimeCandidates) do + if dequeuedCount >= actualMaxCount then break end + tryServe(ckQueueName) +end + +-- Pass 2: fill + discovery in today's age order (work conservation, +-- mixed-deploy safety). Never runs when pass 1 filled the batch. +if dequeuedCount < actualMaxCount then + local ckQueues = redis.call('ZRANGEBYSCORE', ckIndexKey, '-inf', tostring(currentTime), 'LIMIT', 0, window) + for _, ckQueueName in ipairs(ckQueues) do + if dequeuedCount >= actualMaxCount then break end + if not attempted[ckQueueName] then + tryServe(ckQueueName) + end + end +end + +-- NEW: persist floor and refresh TTLs +redis.call('SET', ckVtimeFloorKey, tostring(floor), 'EX', stateTtl) +if redis.call('EXISTS', ckVtimeKey) == 1 then + redis.call('EXPIRE', ckVtimeKey, stateTtl) +end + +-- master queue rebalance: verbatim from the tracked command (uses ckIndex, +-- which keeps its timestamp domain) +local earliestIdx = redis.call('ZRANGE', ckIndexKey, 0, 0, 'WITHSCORES') +if #earliestIdx == 0 then + redis.call('ZREM', masterQueueKey, ckWildcardName) +else + redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) +end + +return results +``` + +Note the `tryServe` extraction is inside the NEW script only; the old script +is not refactored. When writing the real script, inline today's per-candidate +block into `tryServe` exactly (including `decrLengthCounter`, the TTL-member +removal, and both rebalance branches); the sketch elides it to keep the plan +readable, and the byte-identity constraint applies to the OLD script, which +is untouched. + +Call-site switch in `#callDequeueMessagesFromCkQueue` (~line 2206): + +```ts +const result = this.#ckVtimeEnabled + ? await this.redis.dequeueMessagesFromCkQueueVtimeTracked( + ckIndexKey, queueConcurrencyLimitKey, envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, envCurrentConcurrencyKey, + messageKeyPrefix, envQueueKey, masterQueueKey, ttlQueueKey, + lengthCounterKey, + this.keys.ckVtimeKeyFromQueue(ckWildcardQueue), + this.keys.ckVtimeFloorKeyFromQueue(ckWildcardQueue), + ckWildcardQueue, String(Date.now()), + String(this.options.defaultEnvConcurrency), + String(this.options.defaultEnvConcurrencyBurstFactor ?? 1), + this.options.redis.keyPrefix ?? "", String(maxCount), + String(this.#ckVtimeQuantum), String(this.#ckVtimeWindowMultiplier), + String(this.#ckVtimeStateTtl) + ) + : await this.redis.dequeueMessagesFromCkQueueTracked(/* unchanged */); +``` + +Add span attributes on the vtime path: `ck_vtime_enabled: true` plus, from a +small extension of the return shape if desired later, keep it simple now and +only tag the flag. + +Verify: `cd internal-packages/run-engine && pnpm run test ./src/run-queue/tests/ckVtime.test.ts --run` +(tests 1-3, 5-9 pass; 4 passes with the direct-ZADD seeding until Task 4), +then `pnpm run typecheck --filter @internal/run-engine`. + +### Task 4: enqueue registration + +File: `index.ts`. Two new commands, `enqueueMessageCkVtimeTracked` and +`enqueueMessageWithTtlCkVtimeTracked`, `numberOfKeys: 17` (the existing 15 +plus `ckVtimeKey` as KEYS[16] and `ckVtimeFloorKey` as KEYS[17]; the WithTtl +variant is existing-16 plus 2), ARGV extended with `stateTtl`. Script body = +existing tracked script verbatim, plus, in the SLOW PATH ONLY, immediately +after the `-- Rebalance CK index` block: + +```lua +-- Register this variant in the virtual-time index at the floor. NX means an +-- already-advanced tag is never rewound. +local vfloor = redis.call('GET', ckVtimeFloorKey) or '0' +redis.call('ZADD', ckVtimeKey, 'NX', vfloor, queueName) +redis.call('EXPIRE', ckVtimeKey, stateTtl) +``` + +The fast path (direct-to-worker-queue when the variant is empty and capacity +is free) does NOT register or advance; see open decision 1. + +Tests first, in `tests/ckVtime.test.ts`: + +10. "enqueue registers the variant at the current floor with NX": drive the + floor to ~5 via serves, enqueue on a fresh key, assert + `ZSCORE ckVtime ` equals the floor; enqueue a second message on a + key whose tag is 9, assert the tag is still 9. +11. "test 4 now passes end-to-end without direct ZADD seeding" (remove the + seeding from test 4). +12. "fast path leaves vtime state untouched": empty variant, free capacity, + enqueue (fast path fires, returns 1); assert no `ckVtime` entry was + created for it. Then saturate capacity, enqueue again (slow path); + assert registration happened. + +Call-site switches at ~lines 1906 and 1941 pick the vtime variants when +`this.#ckVtimeEnabled`, passing the two extra keys and `stateTtl`. Add both +to the module augmentation. + +Verify: same test file command; plus +`pnpm run test ./src/run-queue/tests/enqueueMessage.test.ts --run` and +`./src/run-queue/tests/ckIndex.test.ts --run` still green (flag off). + +### Task 5: nack registration + +File: `index.ts`. New command `nackMessageCkVtimeTracked`, +`numberOfKeys: 13` (existing 11 plus the two vtime keys), ARGV plus +`stateTtl`. Body = existing verbatim plus the same NX-register block after +its `-- Rebalance CK index` section. Call-site switch at ~line 2584. + +Test first (in `tests/ckVtime.test.ts`): + +13. "nack re-registers a GC'd variant": enqueue one message on `ck: a`, + dequeue it (variant now GC'd from both indexes), nack it; assert the + variant is back in `ckIndex` AND in `ckVtime` at the floor, and a + subsequent dequeue serves it (respecting its future score if the nack + applied backoff: use a nack with an immediate retry score). + +Closure invariant test: + +14. "ckVtime membership tracks ckIndex membership": property-style loop of + ~200 random operations (enqueue on 1 of 8 keys, dequeue batch, ack or + nack a random in-flight message); after each step assert every member of + `ckIndex` is a member of `ckVtime` (the converse may transiently not + hold, which is fine; stale `ckVtime` entries GC on scan). + +Verify: `pnpm run test ./src/run-queue/tests/ckVtime.test.ts --run` and +`./src/run-queue/tests/nack.test.ts --run` (flag off, untouched). + +### Task 6: fairness scenarios on the real batched path + +File: `tests/ckVtimeFairness.test.ts` (new). This closes the spike's fidelity +gap: the spike proved the ordering at `maxCount = 1` with driver-side +rescoring; these tests drive the REAL batched Lua (`maxCount = 10`) with the +state advanced inside the script. + +Harness design (deterministic, no wall-clock sleeps, no spike imports): a +step loop against one `RunQueue` on testcontainers Redis. + +- Enqueue with explicit `timestamp` values in `InputPayload` (all in the + past so everything is time-eligible; the backlog key gets one old shared + timestamp, other keys get strictly increasing later timestamps, mirroring + the ckScenarios head-age reasoning). +- Each step: call the dequeue path once with `maxCount: 10` (via the public + dequeue API used by `ckIndex.test.ts`), record `(step, variant, messageId)` + per served message, then ack each served message after a per-key logical + hold of H steps (keep a small in-flight list and ack entries whose + `servedAt + H <= step`), which is how the env concurrency contends. +- Wait metric per message = serve step minus a per-message logical arrival + step (arrival step derived from the enqueue order). All assertions are on + ratios between flag-on and flag-off runs of the SAME scenario and seed, so + they are stable in CI; use generous factors. + +Scenarios (ported shapes from `ckScenarios.ts` and +`capsFairness.bench.test.ts`, scaled down for CI): + +- ckSkew: heavy 120 backlog msgs (old shared head), 4 light keys x 10 msgs + (later heads). env limit 4, hold 3 steps. Assert: mean light-key wait with + flag ON <= 0.3 x flag OFF (spike measured ~1100 -> ~15, so 0.3 is very + loose); heavy key's wait may rise (do not assert it down). +- ckTrickle: bulk 120, two trickle keys x 15. Same assertion. +- ckSybil (the case caps cannot fix): 20 attacker keys x 8 msgs each, all + older heads, 1 light key x 10 newer. Assert: flag ON mean light wait + <= 0.7 x flag OFF (spike: 1765 -> 1009), AND light key's first serve + happens within the first 3 steps (reachability at the floor), AND + contention-window share: over the steps where >= 2 keys have queued + backlog, light's served fraction >= 0.5 x its fair share 1/21 (directional, + per the spike's confounding caveat; wait is the headline). +- ckBalanced (no-harm check): 4 symmetric keys x 25. Assert: max per-key + mean wait with flag ON <= 1.25 x flag OFF (fair order must not make the + symmetric case worse). +- ckHeavyIdle (work conservation): single key, 60 msgs. Assert: steps to + drain with flag ON == flag OFF exactly (nothing else contends, so any + extra step is a work-conservation bug). + +Also assert in every scenario: total served ON == total served OFF == total +enqueued (no loss, no double-serve; `messageId`s unique). + +Verify: `cd internal-packages/run-engine && pnpm run test ./src/run-queue/tests/ckVtimeFairness.test.ts --run` +(all scenarios pass; target < 60s wall time total, scale message counts down +if needed before loosening assertions). + +### Task 7: multi-consumer / multi-shard correctness + +File: `tests/ckVtimeConcurrency.test.ts` (new). + +15. "two consumers, one base queue, no corruption": one `RunQueue` for + enqueues, two more instances (same Redis, same key prefix, flag on) each + running a dequeue loop with `maxCount: 5` concurrently + (`Promise.all` of two loops, acking with a short hold). 6 keys x 30 + messages. Assert: every message served exactly once across both + consumers (union of served IDs has no duplicates and equals the enqueued + set); after drain, `ckVtime` is empty and floor equals the max it ever + reached; sample the floor between iterations and assert it never + decreased. The correctness argument is that every mutation happens + inside one Lua script and Redis serialises scripts; this test is the + check that the scripts do not assume cross-call state. +16. "concurrent enqueue during dequeue cannot rewind a tag": interleave + enqueues on a hot key with dequeue batches; after each round assert + `ZSCORE ckVtime ` is non-decreasing (NX registration + advance-only + writes). + +17. "op-count budget": using a second plain Redis client, `CONFIG RESETSTAT`, + run 50 identical dequeue calls flag OFF, snapshot + `INFO commandstats` total calls; repeat flag ON with identical data. + Assert `on_total <= off_total + 50 * (6 + 2 * maxCount)` (per call the + vtime path adds at worst: GET floor, ZRANGE min, ZRANGE window, SET + floor, EXPIRE, the pass-2 ZRANGEBYSCORE, plus per serve one ZSCORE and + one ZADD). This pins the per-dequeue overhead the way the caps plan pins + the fairQueue snapshot cost. + +Verify: `cd internal-packages/run-engine && pnpm run test ./src/run-queue/tests/ckVtimeConcurrency.test.ts --run`. + +### Task 8: default-off regression proof + +Location: `tests/ckVtime.test.ts` (final describe block). + +18. "flag off creates no vtime keys and matches today's order": with + `ckVirtualTimeScheduling` absent, run a mixed sequence (enqueues across + 3 keys with distinct head ages, batched dequeues, one nack, acks), then: + `KEYS *` contains no key matching `*ckVtime*`; the dequeue order equals + the head-timestamp order (re-assert the core expectation of + `ckIndex.test.ts` inside this sequence). The stronger guarantee (same + script text, same SHA) holds by construction: the off path calls the + same command names whose `defineCommand` strings this plan never edits; + say so in a comment rather than pretending a test can diff against an + old build. +19. Run the whole existing run-queue suite with the code in place and flag + off: `cd internal-packages/run-engine && pnpm run test ./src/run-queue/ --run` + (excluding the `fairness-spike*` dirs if they are still present). All + green. + +### Task 9: engine and webapp wiring (code-dark rollout) + +Files: `engine/types.ts`, `engine/index.ts`, `apps/webapp/app/env.server.ts`, +`apps/webapp/app/v3/runEngine.server.ts`. + +- `engine/types.ts`, inside `queue:`: + `ckVirtualTimeScheduling?: RunQueueOptions["ckVirtualTimeScheduling"];` +- `engine/index.ts` (~line 196): pass + `ckVirtualTimeScheduling: options.queue?.ckVirtualTimeScheduling,`. +- `env.server.ts`: + `RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED: z.string().default("0")`, + `RUN_ENGINE_CK_VTIME_QUANTUM: z.coerce.number().default(1)`, + `RUN_ENGINE_CK_VTIME_WINDOW_MULTIPLIER: z.coerce.number().default(3)`, + `RUN_ENGINE_CK_VTIME_STATE_TTL_SECONDS: z.coerce.number().default(86400)` + (match the file's existing patterns for flag-style vars). +- `runEngine.server.ts` `queue:` block: + +```ts +ckVirtualTimeScheduling: + env.RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED === "1" + ? { + enabled: true, + quantum: env.RUN_ENGINE_CK_VTIME_QUANTUM, + scanWindowMultiplier: env.RUN_ENGINE_CK_VTIME_WINDOW_MULTIPLIER, + stateTtlSeconds: env.RUN_ENGINE_CK_VTIME_STATE_TTL_SECONDS, + } + : undefined, +``` + +Mixed-deploy analysis to record in the PR description (the analogue of the +caps plan's mixed-arity warning): old and new instances coexist safely +because ioredis registers scripts per process, so arity never mixes within a +script call; old-instance enqueues skip registration and old-instance +dequeues neither advance tags nor GC `ckVtime`. Consequences during overlap: +unregistered variants are served via pass 2 (age order, today's behaviour) +and get registered on serve; keys served by old instances gain a temporary +priority bias (tags lag), which the floor bounds and which disappears when +the rollout completes. No state leaks: `ckVtime` entries created during a +rollout that is then rolled BACK are ignored by the old script entirely and +expire via the state TTL. Turning the flag OFF after running ON is the same: +stale vtime keys are inert and expire within `stateTtlSeconds`. + +Verify: `pnpm run typecheck --filter @internal/run-engine` and +`pnpm run typecheck --filter webapp`. + +### Task 10: ship notes and cleanup + +- Add `.server-changes/2026-07-24-ck-fair-scheduling.md` per + `.server-changes/README.md` (user-facing wording; it ships dark, so the + note says the fair ordering exists behind a flag and changes nothing by + default). No changeset (no public package touched). +- `pnpm run format && pnpm run lint:fix` before committing. +- The `fairness-spike/` and `fairness-spike-ck/` directories say "delete + before any merge to main" in their own headers. Deleting them is a + separate commit/decision, not part of this implementation branch; do not + import from them (already a global constraint). + +## Rollout sequence (after merge) + +1. Deploy with the flag off (nothing changes; scripts for the new commands + are registered but never called). +2. Enable on a staging/test cell; watch dequeue latency spans and Redis op + rates against the Task-7 budget; run a manual sybil-shaped workload and + confirm the light key's wait. +3. Enable in production. During the instance-rolling window the behaviour + interpolates between age order and fair order per the mixed-deploy + analysis; both endpoints are safe. +4. Rollback at any point = flip the env var off; stale vtime keys expire via + TTL within 24h. + +## Open design decisions (flagged, with recommended defaults) + +1. Fast-path enqueue does not advance or register virtual time. + Recommended: keep it that way. The fast path fires only when the variant + queue is empty AND env and queue capacity are free, i.e. when there is no + contention, and fairness only exists under contention. Charging fast-path + serves would need vtime keys touched on the hot uncontended path for no + measurable benefit. Revisit only if a workload alternates fast-path and + queued serves on the same keys at saturation boundaries (the Task-6 + ckBalanced no-harm test would catch a regression shape here). +2. Stored tag semantics and quantum. Recommended: store the NEXT start tag + (start of last serve + quantum), quantum 1.0, matching `SfqCk` in + `disciplines.ts` exactly, since that is the vetted logic both spikes + measured. A cost-proportional quantum (e.g. by machine size) is possible + later via the same field. +3. Pass-1 window multiplier. Recommended: default 3 (today's), configurable. + The residual reachability limit (more than `window` at-cap or + future-scheduled low-tag variants hiding an eligible one) is the same + class as today's `*3` limit and pass 2 keeps the call work-conserving; + widening by default would raise per-call cost for a case not yet observed. +4. Registration sites. Recommended: enqueue and nack only, with pass 2 as + the safety net, per the closure argument (only enqueue and nack make a + variant queue non-empty). Adding registration to the sweeper/ack Luas + would touch more scripts for no covered transition. +5. State TTL default. Recommended: 86400s, matching `counterTtlSeconds`'s + precedent and rationale (periodic re-anchor bounds any drift, including + drift from rolling-deploy overlap). +6. Command variants vs ARGV-gated single script. Recommended: separate + `...Vtime...` commands. Byte-identity when off then holds by construction + instead of by test. +7. Equal weights. Recommended: yes, with the `quantum / weight` hook left in + place (weight fixed at 1, named local, comment pointing at a future + sparse `:ckWeights` HASH shaped like Phase-2's `:ckLimits`). No product + surface for weights exists today. +8. Discipline. Recommended: SFQ (stride is arithmetically the same thing + here; DRR would need a ring cursor in Redis and buys nothing per the + spike, where DRR and SFQ tracked each other within noise). Keep DRR as + the documented O(1) fallback if ZSET ops on `ckVtime` ever show up in + profiles, which the Task-7 op budget makes visible. + +## Verification summary + +```bash +pnpm run typecheck --filter @internal/run-engine +pnpm run typecheck --filter webapp +cd internal-packages/run-engine +pnpm run test ./src/run-queue/tests/keyProducer.test.ts --run +pnpm run test ./src/run-queue/tests/ckVtime.test.ts --run +pnpm run test ./src/run-queue/tests/ckVtimeFairness.test.ts --run +pnpm run test ./src/run-queue/tests/ckVtimeConcurrency.test.ts --run +pnpm run test ./src/run-queue/ --run # full regression, flag off default +``` diff --git a/internal-packages/run-engine/design/references/README.md b/internal-packages/run-engine/design/references/README.md new file mode 100644 index 00000000000..2cd163408cc --- /dev/null +++ b/internal-packages/run-engine/design/references/README.md @@ -0,0 +1,43 @@ +# Run-queue multi-tenant fairness: spike references + +Reference material for the implementation plan +`internal-packages/run-engine/design/plans/2026-07-23-ck-virtual-time-scheduling-plan.md`. These are +the findings and the queueing-theory research produced by three throwaway spikes +on RunQueue tenant fairness (#2617). The spikes' harness, bench, and results code +was throwaway and is NOT on this branch; it is archived on the remote branch +`chore/fair-queueing-spike` (never merged, delete-before-anything). Any +`internal-packages/.../fairness-spike*` paths mentioned inside these documents +refer to that archived code. + +## The documents + +- `run-queue-fairness-research.md` — queueing-theory grounding: SFQ/WFQ and DRR + delay bounds, the Parekh-Gallager result that a worst-case per-flow delay bound + needs BOTH an admission regulator and a scheduler, why CoDel is an AQM and not a + fairness scheduler, and how production systems (Kubernetes APF, YARN, SQL Server + Resource Governor, SQS fair queues) layer caps under a fair order. +- `run-queue-fairness-base-queue-findings.md` — spike 1, base-queue grain: ranked + SFQ / stride / DRR / CoDel against the production age-order baseline. SFQ and + stride fix starvation and are seed-stable; CoDel is a no-op on a fair base and + harmful on an unfair one. +- `run-queue-fairness-ck-findings.md` — spike 2, the real concurrency-key seam: + drove the production `dequeueMessagesFromCkQueueTracked` Lua via `ckIndex` + rescoring. Per-key fairness lives below the selection-strategy interface, in the + CK dequeue scoring; virtual-time ordering fixes it there. Documents the + `maxCount = 1` fidelity limit that the implementation plan's tests must close. +- `run-queue-fairness-caps-vs-scheduling-findings.md` — spike 3, the + reconciliation with the plan of record (which ships concurrency caps): caps and + scheduling are orthogonal knobs. A per-key cap fixes wait when one key floods + but gives no relief once a tenant shards across many keys (the sybil split), and + it is not work-conserving; a total cap is a cross-task knob, not a cross-key + one; fair scheduling fixes every case and stays work-conserving. Ship the caps + first, add the fair order as the general fix, layer them. + +## Why the plan follows from these + +The recommended fix (score `ckIndex` by SFQ virtual time, inside the batched CK +dequeue, layered under the caps) is the one mechanism the spikes found that +survives key-sharding and stays work-conserving, and the research says the caps +the plan of record ships cannot bound wait on their own. The plan turns that into +a flag-gated, mixed-deploy-safe engine change with a test suite that exercises the +real batched path the spikes could not. diff --git a/internal-packages/run-engine/design/references/diagrams/fairness-how-it-works.png b/internal-packages/run-engine/design/references/diagrams/fairness-how-it-works.png new file mode 100644 index 00000000000..98e33bb88a6 Binary files /dev/null and b/internal-packages/run-engine/design/references/diagrams/fairness-how-it-works.png differ diff --git a/internal-packages/run-engine/design/references/diagrams/fairness-problem-and-fix.png b/internal-packages/run-engine/design/references/diagrams/fairness-problem-and-fix.png new file mode 100644 index 00000000000..07027b76c98 Binary files /dev/null and b/internal-packages/run-engine/design/references/diagrams/fairness-problem-and-fix.png differ diff --git a/internal-packages/run-engine/design/references/run-queue-fairness-base-queue-findings.md b/internal-packages/run-engine/design/references/run-queue-fairness-base-queue-findings.md new file mode 100644 index 00000000000..c360f78328f --- /dev/null +++ b/internal-packages/run-engine/design/references/run-queue-fairness-base-queue-findings.md @@ -0,0 +1,168 @@ +# Fair-queueing spike: findings + +Findings from a throwaway spike whose harness is archived and ships nothing; these +findings are retained here as a design reference that informed the change, not as code. + +Read the caveats section before quoting any single number. Two of them matter up +front: (1) the candidate selectors are handed tenant identity (parsed from the +queue name) and the exact per-tenant weights, and the fairness target is defined +by those same groups and weights, so the candidate win over the tenant-blind +baseline is closer to definitional than discovered. (2) The harness anchors +message scores far in the past, which flattens all queue ages, so the baseline's +production age bias is not exercised here; the baseline measured is closer to a +uniform-random shuffle than the real one. + +Bottom line: at the base-queue grain, virtual-time ordering (SFQ) and stride give +tight, seed-stable proportional fairness, honour weights, and cut a starved +tenant's wait hard. DRR lands within noise of them (its small shortfall is a +measurement artifact of the harness, not an intrinsic property). The baseline is +fair on average but seed-variant and has no weight concept. The CoDel wrapper is +not worth shipping as built: it is a forced no-op under bulk arrival and it +actively hurt fairness on the one trickle-arrival workload that could exercise it. +The biggest single result is architectural: per-concurrency-key fairness (the +actual #2617 grain) cannot be expressed through the `RunQueueSelectionStrategy` +interface at all; it lives below that interface, in the CK-dequeue Lua. + +Every number comes from the real `RunQueue` against a testcontainers Redis, one +selector per run, real enqueue/dequeue/ack and real concurrency gating. Each +scenario runs over 3 seeds; tables show the mean and min..max spread. Per-tenant +detail (first seed) is in `results/*.json`. + +## Grain, and why it is not the concurrency key + +A tenant is the fairness group; a tenant owns one or more base queues. The +adversarial scenario gives one tenant 30 queues and the light tenants one each, +which is how the #2617 starvation shows up at the base-queue grain: an ordering +blind to tenant identity lets the many-queue tenant win most of the selection +chances. + +The concurrency-key grain #2617 asks for is not reachable through the strategy +interface. `FairQueueSelectionStrategy` reads the master-queue members verbatim, +and CK runs enqueue a single CK-wildcard entry per base queue. The per-CK pick +runs later inside `dequeueMessagesFromCkQueueTracked`, where `ckIndexKey` is a +ZSET of CK-queues scored by head timestamp and the Lua serves them oldest-first. +That age ordering is the unfairness. Fixing it means changing that Lua or the +`ckIndex` scoring, not the selection strategy. That is the follow-on spike. + +## How fairness is measured (and its limits) + +Because the sim drains every run, final throughput share is fixed by the workload +and cannot tell selectors apart. Two measures do: + +- contention share: a tenant's share of dequeues at instants when at least two + tenants have arrived, unserved work, over its expected weighted share. + `contWorstS/W` is the least-served contender; 1.0 is fair, near 0 means starved + while others had work. Getting this right took two corrections a review caught: + it must only count a tenant once its runs have actually arrived (else poisson + arrival looks like starvation), and the virtual-time floor must be monotonic + (else a returning idle tenant monopolises and skews the window). Even so, when + tenants have very different volumes (trickleStale: 30 runs vs 300) the + low-volume tenant can legitimately be over- or under-represented in the window, + so read this metric together with wait, not alone. +- wait: dequeue time minus enqueue time, per tenant, in the JSON. This is the + clean anti-staleness signal. (Note: `worstWaitP99` in the JSON is NOT an + anti-staleness win signal; it is dominated by the highest-volume tenant, which + a fair selector deliberately delays, so a fairer selector scores worse on it. + Use per-tenant wait.) + +## Results + +`contWorstS/W` mean over 3 seeds (min..max). Higher is fairer. + +| scenario | baseline | sfq | drr | stride | codel-sfq | codel-baseline | +| --------------- | ------------------- | ----- | ------------------- | ------ | --------- | -------------- | +| balanced | 0.889 (0.774..0.954)| 0.985 | 0.954 (0.923..0.970)| 0.985 | 0.985 | 0.889 | +| adversarialSkew | 0.288 (0.261..0.310)| 1.000 | 0.978 (0.968..0.984)| 1.000 | 1.000 | 0.288 | +| weighted | 0.703 (0.679..0.719)| 1.000 | 0.990 (0.977..1.000)| 1.000 | 1.000 | 0.703 | +| burst | 0.978 (0.966..0.992)| 0.992 | 0.958 (0.941..0.975)| 0.992 | 0.992 | 0.978 | +| longHold | 0.828 (0.800..0.842)| 0.981 | 0.981 | 0.981 | 0.981 | 0.828 | +| trickleStale | 0.208 (0.179..0.235)| 0.804 (0.769..0.826)| 0.776 (0.769..0.783)| 0.804 | 0.366 | 0.195 | + +Per-tenant mean wait (seed-a, logical ms), the anti-staleness signal: + +| scenario / selector | low-volume tenant wait | heavy tenant wait | +| ------------------------ | ---------------------- | ----------------- | +| adversarialSkew baseline | 1380 | 805 | +| adversarialSkew sfq | 319 | 1324 | +| trickleStale baseline | 1359 | 1234 | +| trickleStale sfq | 19 | 1353 | +| trickleStale codel-sfq | 213 | 1315 | + +Reading these: the fair selectors cut the light tenant's wait (skew 1380 to 319, +trickle 1359 to 19) by making the heavy tenant wait its fair turn. The heavy +tenant is not punished, it stops jumping the queue. CoDel undoes part of the +trickle win (19 back up to 213). + +## Verdict per mechanism + +- SFQ (start-time virtual time, the start-tag form of WFQ): the strongest result. + Perfect contention fairness under skew and weighting, seed-stable (zero variance + across seeds), and the largest cut to the starved tenant's wait. The floor is + now monotonic (a review found the earlier version let a returning idle tenant + monopolise; fixed). Recommended as the leaf ordering. +- Stride: identical to SFQ to the decimal on every scenario. The spike does not + separate them. Stride carries slightly less state. +- DRR: within noise of SFQ. It trails by a couple of points on balanced (0.954) + and burst (0.958) and matches SFQ elsewhere. That small shortfall is a + measurement artifact, not an intrinsic property: the driver drains a whole + capacity batch from a single strategy snapshot and only advances DRR's deficit + after the batch (via `onServiced`), so DRR's current-winner group, whose queues + it fronts together, grabs several slots before its deficit updates and its + deficit runs negative. Served one-at-a-time DRR is exactly fair (see + `drr.test.ts`). Note the earlier claim that "virtual-time sorts an over-served + group's queues to the back and so avoids this" was wrong: at a tie all of a + group's queues share one clock, so SFQ fronts them together too; the schemes + only separate after their state advances. DRR is O(1) and composes weight + trivially, so it is a fine choice if per-op cost matters, subject to that + caveat. +- CoDel wrapper: do not ship as built. Under bulk arrival it is a forced no-op: + all of a queue's runs share one enqueue timestamp, so every tenant's sojourn is + identical and they all cross the target together, so hoisting everyone collapses + to the base order (this is why codel-sfq equals sfq and codel-baseline equals + baseline to the decimal on those scenarios; it is one workload shape confirming + a null result, not five independent tests). On trickleStale, the one scenario + where sojourns diverge, the sojourn-hoist overshoots: it drops SFQ from 0.804 to + 0.366 and pushes the trickle tenant's wait from 19 back to 213. A staleness + monitor may still help on top of an unfair base or behind a hard concurrency + wall, but that needs a different construction and this spike does not support it. +- Baseline (`FairQueueSelectionStrategy`): fair on average on the easy scenarios + but seed-variant (balanced 0.774..0.954), no weight concept (weighted 0.703), + and it starves a light tenant under queue-count skew (0.288) and under trickle + arrival (0.208, trickle wait 1359). Remember its age bias is not exercised here + (see caveats), so this is a floor on its unfairness, not the production picture. + +## Caveats + +- Grain is base queues, not concurrency keys. The disciplines are grain-agnostic + so the ranking should carry over, but the #2617 gap itself needs the CK-Lua + spike. adversarialSkew is a proxy for that gap, not a measurement of it. +- Definitional advantage: candidates get tenant identity and exact weights the + real interface does not carry; the baseline structurally cannot. +- Baseline age bias inert: scores are anchored ~600s in the past, so all queue + ages are near-equal and the baseline degenerates to near-uniform selection. The + production age bias (which would give a heavy tenant's older heads more weight, + i.e. make skew worse) is not measured. +- Selection-only seam: the driver feeds serviced descriptors back via an + `onServiced` hook; production would advance selector state inside the ack/dequeue + Lua. The spike proves ordering logic, not that wiring. +- Cost was not rigorously measured. `selectionRounds` is roughly equal across + selectors (646..729) but is not comparable between them (a candidate reads all + queues per call; the baseline short-circuits at capacity), and there is no load + benchmark. DRR's "O(1)" advantage is a theory claim, not a spike measurement. +- Scenario quality varies. balanced best shows the baseline's variance; + adversarialSkew and weighted carry the clear separation; longHold and burst + barely separate the candidates; trickleStale's contention number only became + meaningful after two metric fixes and should be read with wait. Per-tenant p99 + equals max for the small (20 to 30 run) tenants, so "p99" there is just the max. +- Single Redis shard; single sequential consumer (not the multi-consumer, + Redis-hash-state design the spec sketched); simulated holds on a logical clock. + Three seeds shows the baseline's variance and the virtual-time schemes' + stability but is not a statistical study. + +## Recommended direction + +Use virtual-time (SFQ, or stride) for leaf ordering and compose weight with it. +DRR is an acceptable O(1) fallback given the batch caveat. Do not adopt the CoDel +wrapper as built. Then run the follow-on spike against the CK-dequeue Lua / +`ckIndex` scoring, because that is where per-tenant fairness actually has to land +in the current design. diff --git a/internal-packages/run-engine/design/references/run-queue-fairness-caps-vs-scheduling-findings.md b/internal-packages/run-engine/design/references/run-queue-fairness-caps-vs-scheduling-findings.md new file mode 100644 index 00000000000..f89257709d3 --- /dev/null +++ b/internal-packages/run-engine/design/references/run-queue-fairness-caps-vs-scheduling-findings.md @@ -0,0 +1,198 @@ +# Caps vs scheduling: reconciliation findings + +Findings from a throwaway spike whose harness is archived (`chore/fair-queueing-spike`) and ships nothing; these findings are retained here as a design reference. Relative ranking +on a small simulation, not a statistical or load study: read the verdicts as "what +this harness supports", not proofs. Went through a blind two-model adversarial +review (a third stalled); the review's fixes are folded in below. + +Bottom line: the plan-of-record's concurrency CAPS and the earlier spike's fair +SCHEDULING are different knobs, and the data on the real CK-dequeue Lua lines up +with the queueing theory (see `RESEARCH.md`). A per-key cap cuts a starved key's +wait when ONE key floods, because Trigger's CK dequeue is oldest-eligible-first; +it gives no wait improvement once a tenant shards its backlog across many +concurrency keys, and it is not work-conserving. Fair scheduling (SFQ/DRR) is the +only knob here that improves the starved key on every scenario including the +sharded one, and it stays work-conserving. A total (per-task) cap is a +cross-task knob; inside one task it only lowers the ceiling and is not a fairness +lever at all. The mechanisms are complementary, and production systems that need +fairness under saturation layer them (Kubernetes APF: seats + fair queueing). + +## How the mechanisms were modelled (fidelity) + +- Per-key cap (Phase 2): the REAL Lua gate. `updateQueueConcurrencyLimits` sets + the base queue's concurrencyLimit; the CK-dequeue Lua caps each ck variant's + in-flight at it and skips an at-limit variant (oldest-eligible-first, true age + order, no rescore). Uniform across variants: Phase 2's per-key HGET override + would cap only the heavy key, but a light key never approaches the cap so the + effect is equivalent here. (So "just lower the existing per-queue concurrency + limit" already IS a per-key cap; Phase 2 makes it per-key-specific.) +- Total cap (Phase 1): driver-side. The real Lua has no group gate yet, so the + driver refuses to admit while total in-flight across all variants of the base + queue (= `:groupConcurrency` SCARD in one base queue) is at the cap. +- Ordering disciplines (baseline age order, SFQ, DRR) are unchanged from the CK + scheduling spike, driven through the same real Lua at `maxCount = 1`. +- Two fidelity limits matter for reading the numbers, both driver-independent: + - `maxCount = 1` (same as the CK spike): production dequeues in batches, so a + real per-key/total gate lives inside the batched Lua. + - The `*3` scan window: the real CK Lua reads `ZRANGEBYSCORE ckIndexKey -inf now + LIMIT 0, actualMaxCount*3` (`index.ts:4041/4193`). At `maxCount = 1` that is + the 3 oldest-scored variants per call. So a per-key cap frees the light key + only when the light head lands inside that 3-wide window after the at-cap + variants ahead of it. With one heavy key it does; with many old-headed + attacker variants it never does. This window governs the skew-works / + sharded-fails split, and it scales with `maxCount` in production (batches), + not fixed at 3, so the sharded result's exact severity would differ on the + real batched path (direction not established). + +## Results + +env=4, per-key cap=2, total cap=2, 3 seeds. Columns: `lightWait` = the starved +key's mean wait (logical ms, the headline where it is not confounded); +`worstWait` = the largest per-group mean wait (i.e. the busiest key, which a fair +discipline deliberately makes wait its turn, so higher here is often correct); +`makespan` = logical time of the last dequeue (a work-conservation signal ONLY on +ckHeavyIdle, arrival-confounded elsewhere); `contWorstS/W` = worst contention +share over weight (directional; volume-confounded and, for per-key cap on sybil, +seed-noisy). + +| scenario | treatment | lightWait | worstWait | makespan | contWorstS/W | +| ----------- | ------------------ | --------- | --------- | -------- | ------------ | +| ckSkew | baseline | 1098 | 1261 | 2083 | 0.187 | +| ckSkew | perKeyCap | 20 | 1555 | 3038 | 0.814 | +| ckSkew | totalCap | 2840 | 2974 | 3947 | 0.213 | +| ckSkew | total+perKey | 2840 | 2974 | 3947 | 0.213 | +| ckSkew | sfq | 14 | 1069 | 2083 | 0.723 | +| ckSkew | drr | 17 | 1067 | 2083 | 0.608 | +| ckSkew | perKeyCap+sfq | 7 | 1628 | 3114 | 0.800 | +| ckSkew | total+perKey+sfq | 52 | 2363 | 3939 | 0.803 | +| ckTrickle | baseline | 1107 | 1134 | 1940 | 0.279 | +| ckTrickle | perKeyCap | 19 | 1555 | 3038 | 0.922 | +| ckTrickle | totalCap | 2852 | 2861 | 3905 | 0.279 | +| ckTrickle | total+perKey | 2852 | 2861 | 3905 | 0.279 | +| ckTrickle | sfq | 17 | 1070 | 1942 | 0.909 | +| ckTrickle | drr | 23 | 1069 | 1941 | 0.790 | +| ckTrickle | perKeyCap+sfq | 8 | 1606 | 3097 | 0.658 | +| ckTrickle | total+perKey+sfq | 106 | 2337 | 3911 | 0.883 | +| ckSybil | baseline | 1765 | 1876 | 2070 | 0.000 | +| ckSybil | perKeyCap | 1776 | 1869 | 2128 | 0.403 | +| ckSybil | totalCap | 3793 | 3801 | 4147 | 0.000 | +| ckSybil | total+perKey | 3793 | 3801 | 4147 | 0.000 | +| ckSybil | sfq | 1009 | 1019 | 2065 | 1.000 | +| ckSybil | drr | 1061 | 1068 | 2055 | 0.994 | +| ckSybil | perKeyCap+sfq | 1010 | 1019 | 2072 | 1.000 | +| ckSybil | total+perKey+sfq | 2292 | 2292 | 4146 | 1.000 | +| ckHeavyIdle | baseline | 633 | 633 | 1240 | 1.000 | +| ckHeavyIdle | perKeyCap | 1291 | 1291 | 2507 | 1.000 | +| ckHeavyIdle | totalCap | 1291 | 1291 | 2507 | 1.000 | +| ckHeavyIdle | total+perKey | 1291 | 1291 | 2507 | 1.000 | +| ckHeavyIdle | sfq | 633 | 633 | 1240 | 1.000 | +| ckHeavyIdle | drr | 633 | 633 | 1240 | 1.000 | +| ckHeavyIdle | perKeyCap+sfq | 1291 | 1291 | 2507 | 1.000 | +| ckHeavyIdle | total+perKey+sfq | 1291 | 1291 | 2507 | 1.000 | + +(ckHeavyIdle is a single key, so "lightWait" is the heavy key's own wait and the +contention metric is degenerate at 1.0; makespan is the signal there. ckSybil is +20 attacker keys plus one light key.) + +## What each mechanism does, at the cross-key grain + +- Per-key cap (Phase 2). SUPPORTED for the single-heavy case; NOT a wait fix once + the tenant shards; not work-conserving. + - Single heavy key (ckSkew/ckTrickle): cuts the light key's wait like a + scheduler (1098 to 20, 1107 to 19) because capping the one heavy key frees + slots and the CK Lua serves the light head as the next eligible one. This + depends on the light head being reachable inside the Lua's 3-wide scan window; + with one at-cap variant ahead of it, it is. + - Sharded / sybil (ckSybil, 20 attacker keys): NO wait improvement (baseline + 1765, perKeyCap 1776; the difference is within the per-seed spread, baseline + 1681..1926, perKeyCap 1672..1861). Contention share nudges up (0.000 to a + mean 0.403) but that mean hides a ~10x seed swing (0.07..0.71), so it is not a + dependable improvement. The reason is structural: the sum of per-key caps is + unbounded relative to the queue when keys are client-chosen, and the 3-wide + scan window is always full of older attacker heads, so the light head is never + reached. Concurrency keys are client-chosen, so this is cheap to trigger. + - Not work-conserving: throttles the capped key even with the env idle + (ckHeavyIdle makespan 1240 to 2507, 2x, the cleanest single result in the + spike; ckSkew 2083 to 3038, though that scenario's makespan is partly arrival- + confounded). +- Total cap (Phase 1) at the cross-key grain: NOT a fairness lever, and the + in-task comparison is capacity-confounded. `totalCap=2` caps the whole task's + aggregate at half of env=4, so it simply halves throughput: light's wait rises + (ckSkew 1098 to 2840) for the same reason heavy's does (both now share half the + server), which is Little's-Law throughput loss, not a fairness effect. It is the + wrong knob for cross-key starvation, measured on a lower ceiling; do not read + the "worse" numbers as "total caps harm fairness." +- Total cap (Phase 1) at the cross-TASK grain: this IS its job, and it works. + Measured in a separate multi-base-queue bench (`crossTaskCaps.bench.test.ts`): + two keyless tasks share one env, a heavy task floods it, and capping the heavy + task (its per-queue concurrency limit, the real native gate, which for a keyless + task equals its total cap) cuts the light TASK's wait from 475 to 2 under the + production `FairQueueSelectionStrategy`. So the total cap protects a light task + from a heavy task, the reservation-isolation role the research describes. It is + still not work-conserving (makespan 2039 to 3039), and SFQ at the task grain + protects the light task too (wait 14) while staying work-conserving (2039). The + fidelity note: this models a KEYLESS task, so the per-queue limit is the total; + a task WITH concurrency keys needs the group SET to sum across variants (the + unbuilt Phase-1 gate). +- Combined total + per-key (the shipped Phase-1+2 config): in this toy the total + cap (2) is below a single per-key cap's reach, so it dominates and the per-key + cap is non-binding (`total+perKey` equals `totalCap` to the digit). This toy + therefore does not exercise the combined config's real regime (total >> per-key, + cross-task). What it does show: adding a fair order on top (`total+perKey+sfq`) + restores fair share within the throttled aggregate (contWorstS/W 0.80..1.0) but + still pays the total cap's throughput loss (makespan ~3900+). +- Scheduling (SFQ/DRR): the only knob that improves the starved key on every + scenario, and work-conserving (makespan stays at the baseline optimum + 2083/1240). On the sharded case SFQ takes the light key from fully starved to + its full fair share (contWorstS/W 0.000 to 1.000, seed-stable) and roughly + halves its wait (1765 to 1009); the residual wait is real saturation shared + fairly across 21 keys, not starvation. SFQ and DRR track each other within + noise, as in the CK spike. +- Layered per-key cap + SFQ: best light-key wait on the single-heavy case (7, 8) + and it carries the cap's occupancy bound, at the cap's makespan cost (matches or + slightly exceeds perKeyCap makespan: ckSkew 3038 to 3114). On the sharded case + the cap adds nothing and SFQ does all the work (1010, same as SFQ alone). This + is the Kubernetes-APF shape; APF avoids the work-conservation cost by making the + cap ELASTIC (borrow/lend seats), which a static cap cannot. + +## Reconciliation with the earlier spike and the plan of record + +Measured here: caps and scheduling fix different things and can be layered +(the per-key-cap+SFQ and total+perKey+sfq rows). Fair scheduling is the only +mechanism in this harness that improves the starved key on the sharded case and +stays work-conserving, which is what the earlier spike recommended (score +`ckIndex` by virtual time). + +Interpretation, NOT measured by this benchmark (it measures wait/makespan/share on +a simulation, not engineering cost or rollout risk): shipping the caps first still +reads as defensible. A per-key cap is bounded, operator-controlled, self-healing +(a Redis SET), and it fully fixes the common single-heavy-key case, which is a +smaller engine change than reworking the dequeue scoring. Its limits are real +(no help once a tenant shards its keys, not work-conserving), which is the case +for treating automatic fair scheduling as a later phase rather than never. + +The layered end state matches Kubernetes APF, SQL Server Resource Governor, YARN, +and the Parekh-Gallager result that a worst-case delay bound needs BOTH an +admission regulator AND a scheduler: keep the caps for isolation and entitlements, +add a fair dequeue order for the contended region when saturation and key-sharding +make caps alone insufficient. Not either/or. + +## Caveats + +- Relative ranking on a simulation; single shard, single base queue, single + sequential consumer; simulated holds on a logical clock; 3 seeds; equal weights. + The verdict words ("supported", "not a wait fix") are relative to this harness. +- `maxCount = 1` and the `*3` scan window (see fidelity section); the total cap is + driver-modelled, not the real (unbuilt) group gate. +- Per-key cap is modelled uniformly (real per-queue gate); a per-key-specific + Phase-2 override is equivalent here only because the light key never approaches + the cap. +- makespan is the last dequeue, not completion, and is arrival-confounded on the + poisson scenarios; trust it only on ckHeavyIdle. +- Contention share is volume-confounded for low-volume keys, and for the per-key + cap on the sharded case it is seed-noisy (0.07..0.71); wait is the trustworthy + signal, share is directional. +- Cross-task isolation (the total cap's real purpose) is now measured in + `crossTaskCaps.bench.test.ts` for KEYLESS tasks (per-queue limit = total cap). + A task with concurrency keys needs the unbuilt group-SET gate to sum across + variants; that batched, keyed path is still not exercised. diff --git a/internal-packages/run-engine/design/references/run-queue-fairness-ck-findings.md b/internal-packages/run-engine/design/references/run-queue-fairness-ck-findings.md new file mode 100644 index 00000000000..04f615c9023 --- /dev/null +++ b/internal-packages/run-engine/design/references/run-queue-fairness-ck-findings.md @@ -0,0 +1,122 @@ +# Per-concurrency-key fairness spike: findings + +Findings from a throwaway spike whose harness is archived (`chore/fair-queueing-spike`) and ships nothing; these findings are retained here as a design reference. + +Bottom line: the base-queue spike's direction carries over to the real seam. At +the concurrency-key grain the production baseline (serve the oldest-head CK +first) starves keys that arrive behind a big backlog, and virtual-time (SFQ) and +stride fix it: they cut the starved key's wait from ~1300ms to ~20ms by making +the backlog key wait its turn. DRR does the same. A CoDel wrapper on the baseline +makes it worse. This was measured by driving the real +`dequeueMessagesFromCkQueueTracked` Lua and only rewriting `ckIndex` scores to +express each discipline. That is enough to say the ordering fix is worth a design +spike, but NOT that a production implementation is proven (see the fidelity +caveat: the spike serves one key per Lua call, and production dequeues in +batches). + +## What was driven, and the two things to know before reading numbers + +Runs enqueue across many concurrency keys under one base queue via the real +`RunQueue`. The per-CK pick is `ZRANGEBYSCORE ckIndexKey -inf now` in the CK Lua +(lowest score first, score = head timestamp). Candidates rewrite those scores +each round to encode discipline order; the baseline leaves them (production age +order). Enqueue, dequeue, concurrency gating and ack all run through the real +code. + +Two caveats a review forced, both load-bearing: + +1. Lead with wait, not contention share. The contention-share metric is + volume-confounded for low-volume keys (a key with 15 runs cannot take a third + of a long window even when served instantly), so on these scenarios it lands + around 0.7 to 0.9 for a discipline that has in fact eliminated the starvation. + The per-key wait is the clean signal. +2. The scenarios must give keys genuinely different head ages. An earlier version + enqueued every run at one timestamp; with tied `ckIndex` scores the real Lua + falls back to a lexicographic member-name tie-break, so the "baseline starves + the heavy key's rivals" result was actually "Redis sorts by name" and the + heavy key only won because "heavy" sorts before "light". Fixed: the backlog + key fires at once (persistently old head) and the other keys arrive via + poisson (distinct, later heads), so the baseline now exercises real age order. + +## Results + +`contWorstS/W` mean over 3 seeds (min..max), and the worst-served key's mean wait +(seed-a, logical ms). Read the wait column as the headline. + +| scenario | discipline | contWorstS/W | worst-key wait | backlog-key wait | +| ---------- | ---------- | ------------------- | -------------- | ---------------- | +| ckSkew | baseline | 0.187 (0.186..0.188)| 1321 | 872 | +| ckSkew | sfq | 0.723 (0.655..0.769)| 16 | 1150 | +| ckSkew | drr | 0.608 (0.556..0.648)| 21 | 1149 | +| ckTrickle | baseline | 0.279 (0.254..0.291)| 1339 | 872 | +| ckTrickle | sfq | 0.909 (0.891..0.918)| 24 | 1237 | +| ckTrickle | drr | 0.790 (0.769..0.818)| 30 | 1236 | + +Full matrix (contWorstS/W mean over 3 seeds): + +| scenario | baseline | sfq | drr | stride | codel(sfq) | codel(baseline) | +| ---------- | ------------------- | ------------------- | ------------------- | ------ | ---------- | ------------------- | +| ckSkew | 0.187 | 0.723 | 0.608 | 0.723 | 0.723 | 0.104 (0.000..0.157)| +| ckBalanced | 0.515 (0.444..0.600)| 0.611 (0.462..0.800)| 0.730 (0.615..0.909)| 0.611 | 0.611 | 0.464 | +| ckTrickle | 0.279 | 0.909 | 0.790 | 0.909 | 0.909 | 0.018 (0.000..0.055)| + +## Verdict per discipline (at the concurrency-key grain) + +- SFQ / stride: fix the starvation. Contention share improves (skew 0.187 to + 0.723, trickle 0.279 to 0.909) and the starved key's wait collapses (skew 1321 + to 16, trickle 1339 to 24) because the backlog key now waits its turn (its wait + rises 872 to ~1150 to 1237). Identical to each other on every scenario. + Recommended discipline for the fix. +- DRR: fixes the wait just as well (skew 21, trickle 30) and its contention share + tracks SFQ within noise (sometimes a little lower, sometimes higher, e.g. + balanced 0.730 vs 0.611). Fine. +- CoDel(sfq): no harm, matches SFQ to the decimal. Adds nothing on top of a fair + base. +- CoDel(baseline): harmful. Hoisting stale keys on top of the age-order baseline + drove ckSkew to 0.104 (below baseline's 0.187) and ckTrickle to 0.018 (below + 0.279). A staleness monitor is not a substitute for a fair base. +- Baseline (production age order): starves keys that queue behind a backlog + (ckSkew 0.187, worst key waits 1321ms; ckTrickle 0.279, 1339ms). It is roughly + fair when keys are symmetric (ckBalanced 0.515, though seed-variant 0.444 to + 0.600). This is the #2617 dynamic at the seam where it lives. + +## Fidelity caveat (the reason this is not "proven for production") + +The driver dequeues one key per Lua call (`maxCount = 1`) and rescores `ckIndex` +before each call. Production dequeues in batches (`maxCount` default 10). Inside a +batched CK-dequeue call the Lua re-scores each served key back to its head +timestamp as it goes, so a once-per-round rescore would only steer the FIRST pick +of a batch; the rest would follow head-timestamp order again. So this spike +demonstrates the ordering fix only in a one-key-per-call regime, which is not how +production dequeues. A real fix has to advance per-key discipline state inside the +Lua on every serve (and hold that state in Redis, not process memory). This spike +does not exercise that batch path, so the correct claim is "the ordering fix is +worth a design spike", not "a production fix is viable". + +## Other caveats + +- Contention share is volume-confounded (see above); the wait column is the + trustworthy signal, and the contention numbers should be read as directional. +- The DRR contention-share gap is NOT the base-queue spike's batch-drain artifact + (that harness batched; this one serves one key per call and advances DRR's + deficit every serve). The cause of DRR's slightly lower share here is not + established; its wait result is as good as SFQ's. +- Per-CK concurrency gating never binds in these runs (no per-CK limit is set, so + it collapses to the env limit), so the spike says nothing about the per-CK + concurrency-limit-multiplication half of #2617, which is out of scope. +- A rescore discipline advances its floor/ring state on the final no-op drain + round of an instant (order() is called before the empty dequeue). It is + self-correcting and does not corrupt the event-based metrics, but it is a minor + infidelity to a production per-serve advance. +- Equal weights only; single shard, single base queue, single sequential + consumer; simulated holds on a logical clock; 3 seeds. + +## Recommended direction + +Score `ckIndex` by a fair discipline (SFQ/stride virtual time, or DRR) instead of +by head timestamp. Both spikes agree on the discipline and this one shows the +ordering fix works through the real dequeue path at `maxCount = 1`. The design +spike past this needs to: advance per-key virtual-time state inside the batched +CK-dequeue Lua (the `maxCount > 1` path this spike did not exercise), hold that +state in Redis for the multi-consumer case, and address the per-CK +concurrency-limit multiplication that is the other half of #2617. diff --git a/internal-packages/run-engine/design/references/run-queue-fairness-research.md b/internal-packages/run-engine/design/references/run-queue-fairness-research.md new file mode 100644 index 00000000000..afb831de568 --- /dev/null +++ b/internal-packages/run-engine/design/references/run-queue-fairness-research.md @@ -0,0 +1,133 @@ +# Queue-fairness research (grounding for the caps-vs-scheduling reconciliation) + +Research notes distilled from a throwaway spike, retained here as a design reference. Five Fable research passes, distilled. Citations kept so +the findings write-up and report can point at real sources. This grounds the +central claim: occupancy caps and fair scheduling are orthogonal knobs, and the +plan-of-record ships the cap knob while the earlier spike measured the scheduler +knob. + +## The orthogonality result (theory) + +Caps bound occupancy, not wait. A tenant capped at C in-flight with mean service +time S has long-run throughput <= C/S (Little's Law, L = lambda*W). That is an +upper bound on the capped tenant's share; it reserves no lower bound for anyone +else and says nothing about any tenant's waiting time (Little relates averages in +a stable system, not tails, and if the capped tenant's arrival rate exceeds C/S +its queue never stabilises so the law does not even apply to it). + +Scheduling bounds wait, not occupancy. WFQ/PGPS tracks GPS within one max job +(Parekh-Gallager finish-time bound L_max/r); SFQ gives a starved flow's head item +a hard wait bound of "one max-size job from every other active tenant" (Goyal-Vin +SFQ Theorem 2), with no server-rate assumption. But all of family A is +work-conserving: a lone backlogged tenant takes 100% of the server. Nothing in a +scheduler limits how many slots a tenant holds. + +Parekh-Gallager is the canonical joint statement: a worst-case per-flow delay +bound is the product of arrival regulation (leaky/token bucket = the admission +knob) AND a scheduling discipline (GPS/WFQ = the order knob). Neither alone yields +the bound. Cruz network-calculus caveat: plain FIFO does get a delay bound IF every +input is burstiness-constrained (arrival regulator on ingress) and aggregate rho < +C, but a concurrency cap is not an ingress regulator (it bounds in-flight, not +queue admission, and queue depth stays unbounded), so under adversarial arrival +FIFO wait is unbounded and the orthogonality holds without qualification. + +Sources: Little 1961 (Oper. Res. 9:383-387); Parekh & Gallager 1993/1994 (GPS, +IEEE/ACM ToN); Goyal, Vin & Cheng, Start-time Fair Queueing (SIGCOMM'96 / ToN'97); +Shreedhar & Varghese, DRR (SIGCOMM'95); Waldspurger & Weihl, Stride Scheduling +(MIT TM-528, 1995); Cruz, A Calculus for Network Delay (IEEE T-IT 1991); Kingman's +formula; Harchol-Balter, Performance Modeling and Design of Computer Systems (CUP +2013). + +## When a cap alone DOES cut a starved tenant's wait (the load-bearing condition) + +A per-tenant concurrency cap on heavy tenant H cuts light tenant L's wait to +near-zero iff BOTH: + +1. Slot availability: sum of caps of all backlogged tenants other than L is < N + (the binding aggregate limit), so freed slots exist that capped tenants can + never occupy; and +2. Eligibility-aware serve order: the dequeue selects the oldest ELIGIBLE item, + skipping items whose tenant is at cap, so L's head is reachable without + draining H's older items first. + +If (2) fails (single global age-ordered list with a head-blocking consumer), the +cap does NOT help L and with strict head-blocking makes L's wait WORSE: H's +backlog drains at k slots instead of N while the freed N-k slots sit idle +(head-of-line blocking; Parekh-Gallager's FCFS-gives-no-isolation remark). + +Trigger's CK dequeue is oldest-ELIGIBLE-first: the CK Lua gates each variant at +its per-key concurrencyLimit and skips a variant that is at its limit, moving to +the next-oldest eligible. So condition (2) holds structurally. That is WHY per-key +caps can work for wait here, and would not work on a head-blocking FIFO. + +## Where caps fail even with eligibility-aware order (the sybil split) + +Concurrency keys are client-chosen. A heavy tenant spreads its backlog across many +CK variants, each under its own per-key cap, all "eligible". The binding +constraint becomes the base-queue total cap (or env limit); oldest-first then +serves the adversary's older backlog across its many keys before a newcomer's +head. Per-key caps bound nothing in aggregate, because sum of per-key caps is +unbounded relative to the queue cap when keys are dynamic. The light tenant's wait +scales with the adversary's total queued backlog, which no per-key cap regulates. +Within a base queue, the fix under adversarial arrival is an order change +(round-robin / fair queueing across keys), not another cap. + +## Known static-cap failure modes (practice) + +2DFQ (Mace et al., SIGCOMM 2016): "Rate limiters, typically implemented as token +buckets, are not designed to provide fairness at short time intervals ... they can +either underutilize the system or concurrent bursts can overload it without +providing any further fairness guarantees." Their desirable-properties section +requires the scheduler be work-conserving, which "precludes the use of ad-hoc +throttling mechanisms to control misbehaving tenants." Pisces (OSDI 2012) and DRF +(NSDI 2011) both exist because static slot partitioning under/over-utilises under +skewed demand. Netflix concurrency-limits: static limits (Limit = RPS * latency, +i.e. Little's Law) "quickly go out of date"; hence adaptive. + +The price of caps when they DO give order-independent wait bounds: they degenerate +into a static partition (sum of caps <= K), which is non-work-conserving +(utilisation ceiling of sum-of-caps even when one tenant could use all K) and +needs bounded, pre-known tenant cardinality. + +## Production precedent: caps and scheduling are LAYERED, not either/or + +- Kubernetes API Priority & Fairness (the closest analog): total server + concurrency split into per-priority-level "seats" (a cap), THEN shuffle-sharded + fair queueing decides dispatch order within a level. Kubernetes hit exactly the + cap-alone failure with max-inflight before APF, and the fix ADDED fair queueing + on top of the existing cap rather than replacing it. (K8s docs; KEP-1040.) +- SQL Server Resource Governor: pool MIN/MAX/CAP percent (caps + reservation) + + workload-group IMPORTANCE biasing the scheduler's order. Same shape. +- YARN Fair/Capacity scheduler: minShare floor + maxResources cap + weighted + fair-share ordering, with preemption to reclaim the floor. +- Mesos/DRF, Borg: quota/admission caps layered with fair-share or priority order. +- Amazon SQS fair queues: fairness metric is DWELL TIME, fixed by reprioritising + delivery ORDER when a tenant's in-flight share is disproportionate, and + explicitly does NOT rate-limit per tenant. Order is the dwell-time knob; + occupancy limits alone were not it. +- Envoy / gRPC / Postgres connection caps: caps ALONE, and they claim overload + PROTECTION, not fairness. AWS token-bucket quotas claim "fairness" only in the + weaker selective-throttling sense, and rely on an elastic, rarely-saturated + fleet. + +Condition for caps-alone to suffice in practice: sum of caps comfortably below (or +elastically kept below) real capacity, and shed/rejected work acceptable, i.e. the +system never holds a contended backlog it must drain in some order. The moment you +hold a queue of admitted-but-waiting work at saturation, the serve order IS the +fairness policy. + +## CoDel (confirms the prior spike's "CoDel disproven" verdict) + +CoDel is an AQM: it bounds standing-queue delay by DROPPING packets when the +minimum sojourn over a window stays above target (5ms/100ms defaults; RFC 8289). +It is not a scheduler and gives no inter-flow fairness (RFC 7567: queue management +and scheduling are complementary, not substitutes). FQ-CoDel gets all its fairness +from a DRR scheduler; CoDel is only the per-flow AQM inside each sub-queue (RFC +8290). In a durable run queue that can never drop work, CoDel's actuator is gone: +reordering conserves total queue and total work, so hoisting one item's sojourn +down pushes others' up. Hoisting the stalest item to the front of an already +age-ordered queue re-applies the age bias the base already has, so it is a no-op +at best and a dominant-tenant amplifier at worst (a tenant that dumps a big +backlog owns the entire stale set). Facebook's server-side CoDel adaptation ("Fail +at Scale", ACM Queue 2015) keeps the drop (stale requests expire) and reorders +adaptive-LIFO (newest first), the opposite of stalest-first hoisting. diff --git a/internal-packages/run-engine/src/engine/index.ts b/internal-packages/run-engine/src/engine/index.ts index 3c1f4330a0f..1db8ec92611 100644 --- a/internal-packages/run-engine/src/engine/index.ts +++ b/internal-packages/run-engine/src/engine/index.ts @@ -234,6 +234,7 @@ export class RunEngine { workerItemsSuffix: "ttl-worker:{queue:ttl-expiration:}items", visibilityTimeoutMs: options.queue?.ttlSystem?.visibilityTimeoutMs ?? 30_000, }, + ckVirtualTimeScheduling: options.queue?.ckVirtualTimeScheduling, }); this.worker = new Worker({ diff --git a/internal-packages/run-engine/src/engine/types.ts b/internal-packages/run-engine/src/engine/types.ts index bb1d6eb2fa9..902d61ed485 100644 --- a/internal-packages/run-engine/src/engine/types.ts +++ b/internal-packages/run-engine/src/engine/types.ts @@ -16,6 +16,7 @@ import { } from "@trigger.dev/redis-worker"; import type { ControlPlaneResolver } from "./controlPlaneResolver.js"; import type { FairQueueSelectionStrategyOptions } from "../run-queue/fairQueueSelectionStrategy.js"; +import type { RunQueueOptions } from "../run-queue/index.js"; import type { MinimalAuthenticatedEnvironment } from "../shared/index.js"; import type { LockRetryConfig } from "./locking.js"; import type { workerCatalog } from "./workerCatalog.js"; @@ -123,6 +124,9 @@ export type RunEngineOptions = { /** Max time (ms) to wait for more items before flushing a batch (default: 5000) */ batchMaxWaitMs?: number; }; + /** Fair (virtual-time) ordering across concurrency-key variants of a base queue. + * Passed through to RunQueue; off by default (undefined = today's behaviour). */ + ckVirtualTimeScheduling?: RunQueueOptions["ckVirtualTimeScheduling"]; }; runLock: { redis: RedisOptions; diff --git a/internal-packages/run-engine/src/run-queue/CK_VTIME_KNOWN_LIMITATIONS.md b/internal-packages/run-engine/src/run-queue/CK_VTIME_KNOWN_LIMITATIONS.md new file mode 100644 index 00000000000..43a6cd43308 --- /dev/null +++ b/internal-packages/run-engine/src/run-queue/CK_VTIME_KNOWN_LIMITATIONS.md @@ -0,0 +1,70 @@ +# CK virtual-time scheduling: known limitations (read before enabling) + +The feature ships behind `RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED` (off by default). +A three-model blind adversarial review found no Critical issues; the correctness +and safety fixes it surfaced are applied. The items below are the review findings +that were deliberately NOT code-fixed because they are bounded, self-healing, or +pre-existing. They are the checklist for the "enable in production" decision. + +## Bounded state drift on paths that don't GC `ckVtime` + +The vtime dequeue command GCs a drained variant from both `ckIndex` and `ckVtime`. +But `acknowledgeMessageCkTracked`, `expireTtlRuns`, `moveToDeadLetterQueueCkTracked`, +and the flag-off dequeue command do NOT remove a drained variant from `ckVtime` +(they were left byte-identical). Consequences, all bounded: + +- A low-tag tombstone (a variant emptied by ack/TTL/DLQ without a vtime serve) is + the minimum entry, so the very next vtime dequeue visits it first, finds the + queue empty, and GCs it: self-heals in ~1 call. It can pin the floor low for + that one call. +- A high-tag tombstone (a heavily-served variant whose remaining backlog is then + removed out-of-band) lingers until the floor climbs to its tag or the 24h state + TTL fires. Pure memory drift, does not affect fairness. +- Rollback (flag on -> off): variants drained by the old command leave inert + `ckVtime` entries. Old code never reads them; they expire within `stateTtl` + (default 24h) once the base queue stops receiving writes. To reclaim sooner, + delete the `*:ckVtime` / `*:ckVtimeFloor` keys after disabling. + +A full fix (vtime-aware ack/TTL/DLQ command variants) is deferred: it adds three +more command variants for a bounded, self-healing drift on a dark feature. + +## Tie-break among equal virtual-time tags is member-name order + +When variants tie at the same tag (a fresh batch at the floor: cold start, new +deploy, or a GC'd variant re-entering), pass 1's `ZRANGE ckVtime` falls back to +Redis's lexicographic member order, i.e. the fully-qualified queue name including +the client-chosen concurrency key. A lex-early name gets a first-serve head start +in a tie. This is PRE-EXISTING (the head-timestamp baseline ties the same way) and +bounded: tags diverge after the first serve, so it affects only first-serve order, +not long-run fairness. A future improvement is to tie-break by head age instead of +member name. Do not rank fairness on an untrusted string if that head start ever +matters at scale. + +## Future-scheduled / retry-backoff variants occupy pass-1 window slots + +Enqueue and nack register a variant in `ckVtime` even when its head message is +scheduled in the future (delayed run, nack backoff). Pass 1 selects by tag with no +readiness filter, so a burst of future-headed variants can fill the pass-1 window +(`maxCount * scanWindowMultiplier`, default 3x); actual serves then come from pass +2 (today's age order). Work conservation still holds (pass 2 is a superset), so +this is fairness degradation under a retry storm, not loss. Widen +`scanWindowMultiplier` if observed. + +## Minor operational notes + +- Idle-polling a CK queue whose only work is future-scheduled now does a couple of + extra Redis writes per poll (floor SET + EXPIRE) vs the old early-return. Bounded; + visible in Redis write metrics after enabling. +- `descriptorFromQueue` positional parsing mis-splits a concurrency key containing + a literal `:` (pre-existing; not introduced here). The vtime feature uses the + full queue key as the ZSET member, which is unaffected, but any code that parses + the member back into fields inherits the pre-existing limitation. + +## Rollout (from the plan) + +1. Deploy with the flag off (new command scripts registered, never called). +2. Enable on a staging cell; watch dequeue-latency spans and Redis op rates against + the op-count budget; run a sybil-shaped workload and confirm the light key's wait. +3. Enable in production; during the instance-rolling window behaviour interpolates + between age order and fair order (both endpoints safe). +4. Rollback = flip the env var off; stale vtime keys expire via TTL within 24h. diff --git a/internal-packages/run-engine/src/run-queue/index.ts b/internal-packages/run-engine/src/run-queue/index.ts index a0571206538..70fe414bc85 100644 --- a/internal-packages/run-engine/src/run-queue/index.ts +++ b/internal-packages/run-engine/src/run-queue/index.ts @@ -118,6 +118,21 @@ export type RunQueueOptions = { /** Visibility timeout for TTL worker jobs (ms, default: 30000) */ visibilityTimeoutMs?: number; }; + /** + * Fair (virtual-time / SFQ) ordering across concurrency-key variants of a + * base queue. Off by default; when off, the exact pre-existing Lua commands + * run and no vtime keys are created. See the CK virtual-time scheduling + * design for the ordering model. + */ + ckVirtualTimeScheduling?: { + enabled: boolean; + /** Virtual-time advance per serve (dimensionless). Default 1. */ + quantum?: number; + /** Pass-1 candidate window = actualMaxCount * this. Default 3. */ + scanWindowMultiplier?: number; + /** EXPIRE applied to ckVtime/ckVtimeFloor on every write. Default 86400. */ + stateTtlSeconds?: number; + }; }; export interface ConcurrencySweeperCallback { @@ -202,10 +217,28 @@ export class RunQueue { private _observableWorkerQueues: Set = new Set(); private _meter: Meter; private _queueCooloffStates: Map = new Map(); + readonly #ckVtimeEnabled: boolean; + readonly #ckVtimeQuantum: number; + readonly #ckVtimeWindowMultiplier: number; + readonly #ckVtimeStateTtl: number; constructor(public readonly options: RunQueueOptions) { this.shardCount = options.shardCount ?? 2; this.counterTtlSeconds = options.counterTtlSeconds ?? 86400; + this.#ckVtimeEnabled = options.ckVirtualTimeScheduling?.enabled ?? false; + // Defense-in-depth: clamp so a directly-constructed RunQueue can't get bad + // values that would freeze tags (quantum <= 0) or force an O(N) scan / + // EX 0 error (multiplier / ttl <= 0). + const resolvedQuantum = options.ckVirtualTimeScheduling?.quantum ?? 1; + this.#ckVtimeQuantum = resolvedQuantum > 0 ? resolvedQuantum : 1; + this.#ckVtimeWindowMultiplier = Math.max( + 1, + Math.floor(options.ckVirtualTimeScheduling?.scanWindowMultiplier ?? 3) + ); + this.#ckVtimeStateTtl = Math.max( + 1, + Math.floor(options.ckVirtualTimeScheduling?.stateTtlSeconds ?? 86400) + ); this.retryOptions = options.retryOptions ?? defaultRetrySettings; this.redis = createRedisClient(options.redis, { onError: (error) => { @@ -1903,72 +1936,145 @@ export class RunQueue { const ckKeyPrefix = this.options.redis.keyPrefix ?? ""; if (ttlInfo) { - result = await this.redis.enqueueMessageWithTtlCkTracked( - // keys - masterQueueKey, - queueKey, - messageKey, - queueCurrentConcurrencyKey, - envCurrentConcurrencyKey, - queueCurrentDequeuedKey, - envCurrentDequeuedKey, - envQueueKey, - ttlInfo.ttlQueueKey, - ckIndexKey, - workerQueueKey, - queueConcurrencyLimitKey, - envConcurrencyLimitKey, - envConcurrencyLimitBurstFactorKey, - lengthCounterKey, - baseQueueKey, - // args - queueName, - messageId, - messageData, - messageScore, - ttlInfo.ttlMember, - String(ttlInfo.ttlExpiresAt), - ckWildcardName, - messageKeyValue, - defaultEnvConcurrencyLimit, - defaultEnvConcurrencyBurstFactor, - currentTime, - enableFastPathArg, - ckKeyPrefix, - String(this.counterTtlSeconds) - ); + result = this.#ckVtimeEnabled + ? await this.redis.enqueueMessageWithTtlCkVtimeTracked( + // keys + masterQueueKey, + queueKey, + messageKey, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ttlInfo.ttlQueueKey, + ckIndexKey, + workerQueueKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + lengthCounterKey, + baseQueueKey, + this.keys.ckVtimeKeyFromQueue(message.queue), + this.keys.ckVtimeFloorKeyFromQueue(message.queue), + // args + queueName, + messageId, + messageData, + messageScore, + ttlInfo.ttlMember, + String(ttlInfo.ttlExpiresAt), + ckWildcardName, + messageKeyValue, + defaultEnvConcurrencyLimit, + defaultEnvConcurrencyBurstFactor, + currentTime, + enableFastPathArg, + ckKeyPrefix, + String(this.counterTtlSeconds), + String(this.#ckVtimeStateTtl) + ) + : await this.redis.enqueueMessageWithTtlCkTracked( + // keys + masterQueueKey, + queueKey, + messageKey, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ttlInfo.ttlQueueKey, + ckIndexKey, + workerQueueKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + lengthCounterKey, + baseQueueKey, + // args + queueName, + messageId, + messageData, + messageScore, + ttlInfo.ttlMember, + String(ttlInfo.ttlExpiresAt), + ckWildcardName, + messageKeyValue, + defaultEnvConcurrencyLimit, + defaultEnvConcurrencyBurstFactor, + currentTime, + enableFastPathArg, + ckKeyPrefix, + String(this.counterTtlSeconds) + ); } else { - result = await this.redis.enqueueMessageCkTracked( - // keys - masterQueueKey, - queueKey, - messageKey, - queueCurrentConcurrencyKey, - envCurrentConcurrencyKey, - queueCurrentDequeuedKey, - envCurrentDequeuedKey, - envQueueKey, - ckIndexKey, - workerQueueKey, - queueConcurrencyLimitKey, - envConcurrencyLimitKey, - envConcurrencyLimitBurstFactorKey, - lengthCounterKey, - baseQueueKey, - // args - queueName, - messageId, - messageData, - messageScore, - ckWildcardName, - messageKeyValue, - defaultEnvConcurrencyLimit, - defaultEnvConcurrencyBurstFactor, - currentTime, - enableFastPathArg, - ckKeyPrefix, - String(this.counterTtlSeconds) - ); + result = this.#ckVtimeEnabled + ? await this.redis.enqueueMessageCkVtimeTracked( + // keys + masterQueueKey, + queueKey, + messageKey, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ckIndexKey, + workerQueueKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + lengthCounterKey, + baseQueueKey, + this.keys.ckVtimeKeyFromQueue(message.queue), + this.keys.ckVtimeFloorKeyFromQueue(message.queue), + // args + queueName, + messageId, + messageData, + messageScore, + ckWildcardName, + messageKeyValue, + defaultEnvConcurrencyLimit, + defaultEnvConcurrencyBurstFactor, + currentTime, + enableFastPathArg, + ckKeyPrefix, + String(this.counterTtlSeconds), + String(this.#ckVtimeStateTtl) + ) + : await this.redis.enqueueMessageCkTracked( + // keys + masterQueueKey, + queueKey, + messageKey, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ckIndexKey, + workerQueueKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + lengthCounterKey, + baseQueueKey, + // args + queueName, + messageId, + messageData, + messageScore, + ckWildcardName, + messageKeyValue, + defaultEnvConcurrencyLimit, + defaultEnvConcurrencyBurstFactor, + currentTime, + enableFastPathArg, + ckKeyPrefix, + String(this.counterTtlSeconds) + ); } } else if (ttlInfo) { // Use the TTL-aware enqueue that atomically adds to both queues @@ -2203,26 +2309,56 @@ export class RunQueue { const lengthCounterKey = this.keys.queueLengthCounterKeyFromQueue(ckWildcardQueue); - const result = await this.redis.dequeueMessagesFromCkQueueTracked( - //keys - ckIndexKey, - queueConcurrencyLimitKey, - envConcurrencyLimitKey, - envConcurrencyLimitBurstFactorKey, - envCurrentConcurrencyKey, - messageKeyPrefix, - envQueueKey, - masterQueueKey, - ttlQueueKey, - lengthCounterKey, - //args - ckWildcardQueue, - String(Date.now()), - String(this.options.defaultEnvConcurrency), - String(this.options.defaultEnvConcurrencyBurstFactor ?? 1), - this.options.redis.keyPrefix ?? "", - String(maxCount) - ); + if (this.#ckVtimeEnabled) { + span.setAttribute("ck_vtime_enabled", true); + } + + const result = this.#ckVtimeEnabled + ? await this.redis.dequeueMessagesFromCkQueueVtimeTracked( + //keys + ckIndexKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + envCurrentConcurrencyKey, + messageKeyPrefix, + envQueueKey, + masterQueueKey, + ttlQueueKey, + lengthCounterKey, + this.keys.ckVtimeKeyFromQueue(ckWildcardQueue), + this.keys.ckVtimeFloorKeyFromQueue(ckWildcardQueue), + //args + ckWildcardQueue, + String(Date.now()), + String(this.options.defaultEnvConcurrency), + String(this.options.defaultEnvConcurrencyBurstFactor ?? 1), + this.options.redis.keyPrefix ?? "", + String(maxCount), + String(this.#ckVtimeQuantum), + String(this.#ckVtimeWindowMultiplier), + String(this.#ckVtimeStateTtl) + ) + : await this.redis.dequeueMessagesFromCkQueueTracked( + //keys + ckIndexKey, + queueConcurrencyLimitKey, + envConcurrencyLimitKey, + envConcurrencyLimitBurstFactorKey, + envCurrentConcurrencyKey, + messageKeyPrefix, + envQueueKey, + masterQueueKey, + ttlQueueKey, + lengthCounterKey, + //args + ckWildcardQueue, + String(Date.now()), + String(this.options.defaultEnvConcurrency), + String(this.options.defaultEnvConcurrencyBurstFactor ?? 1), + this.options.redis.keyPrefix ?? "", + String(maxCount) + ); if (!result) { span.setAttribute("message_count", 0); @@ -2581,28 +2717,56 @@ export class RunQueue { const lengthCounterKey = this.keys.queueLengthCounterKeyFromQueue(message.queue); const runningCounterKey = this.keys.queueRunningCounterKeyFromQueue(message.queue); - await this.redis.nackMessageCkTracked( - //keys - masterQueueKey, - messageKey, - messageQueue, - queueCurrentConcurrencyKey, - envCurrentConcurrencyKey, - queueCurrentDequeuedKey, - envCurrentDequeuedKey, - envQueueKey, - ckIndexKey, - lengthCounterKey, - runningCounterKey, - //args - messageId, - messageQueue, - JSON.stringify(message), - String(messageScore), - ckWildcardName, - this.options.redis.keyPrefix ?? "", - String(this.counterTtlSeconds) - ); + if (this.#ckVtimeEnabled) { + await this.redis.nackMessageCkVtimeTracked( + //keys + masterQueueKey, + messageKey, + messageQueue, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ckIndexKey, + lengthCounterKey, + runningCounterKey, + this.keys.ckVtimeKeyFromQueue(message.queue), + this.keys.ckVtimeFloorKeyFromQueue(message.queue), + //args + messageId, + messageQueue, + JSON.stringify(message), + String(messageScore), + ckWildcardName, + this.options.redis.keyPrefix ?? "", + String(this.counterTtlSeconds), + String(this.#ckVtimeStateTtl) + ); + } else { + await this.redis.nackMessageCkTracked( + //keys + masterQueueKey, + messageKey, + messageQueue, + queueCurrentConcurrencyKey, + envCurrentConcurrencyKey, + queueCurrentDequeuedKey, + envCurrentDequeuedKey, + envQueueKey, + ckIndexKey, + lengthCounterKey, + runningCounterKey, + //args + messageId, + messageQueue, + JSON.stringify(message), + String(messageScore), + ckWildcardName, + this.options.redis.keyPrefix ?? "", + String(this.counterTtlSeconds) + ); + } } else { await this.redis.nackMessage( //keys @@ -3650,71 +3814,334 @@ return 0 `, }); - // Expire TTL runs - atomically removes from TTL set, acknowledges from normal queue, and enqueues to TTL worker - this.redis.defineCommand("expireTtlRuns", { - numberOfKeys: 1, + // Vtime variant of enqueueMessageCkTracked (feature-flagged via + // ckVirtualTimeScheduling.enabled). Identical script body, plus slow-path + // registration of the variant into the :ckVtime ZSET at the floor (NX), so a + // brand-new key is present in the fair order from its first enqueue. The + // fast path (direct-to-worker-queue) neither registers nor advances. + this.redis.defineCommand("enqueueMessageCkVtimeTracked", { + numberOfKeys: 17, lua: ` -local ttlQueueKey = KEYS[1] -local keyPrefix = ARGV[1] -local currentTime = tonumber(ARGV[2]) -local batchSize = tonumber(ARGV[3]) -local shardCount = tonumber(ARGV[4]) -local workerQueueKey = ARGV[5] -local workerItemsKey = ARGV[6] -local visibilityTimeoutMs = tonumber(ARGV[7]) +local masterQueueKey = KEYS[1] +local queueKey = KEYS[2] +local messageKey = KEYS[3] +local queueCurrentConcurrencyKey = KEYS[4] +local envCurrentConcurrencyKey = KEYS[5] +local queueCurrentDequeuedKey = KEYS[6] +local envCurrentDequeuedKey = KEYS[7] +local envQueueKey = KEYS[8] +local ckIndexKey = KEYS[9] +-- Fast-path keys (KEYS 10-13) +local workerQueueKey = KEYS[10] +local queueConcurrencyLimitKey = KEYS[11] +local envConcurrencyLimitKey = KEYS[12] +local envConcurrencyLimitBurstFactorKey = KEYS[13] +-- Counter keys (KEYS 14-15) +local lengthCounterKey = KEYS[14] +local baseQueueKey = KEYS[15] +-- Virtual-time keys (KEYS 16-17) +local ckVtimeKey = KEYS[16] +local ckVtimeFloorKey = KEYS[17] --- Get expired runs from TTL sorted set (score <= currentTime) -local expiredMembers = redis.call('ZRANGEBYSCORE', ttlQueueKey, '-inf', currentTime, 'LIMIT', 0, batchSize) +local queueName = ARGV[1] +local messageId = ARGV[2] +local messageData = ARGV[3] +local messageScore = ARGV[4] +local ckWildcardName = ARGV[5] +-- Fast-path args (ARGV 6-10) +local messageKeyValue = ARGV[6] +local defaultEnvConcurrencyLimit = ARGV[7] +local defaultEnvConcurrencyBurstFactor = ARGV[8] +local currentTime = ARGV[9] +local enableFastPath = ARGV[10] +-- keyPrefix for prepending to variant names stored as values in ckIndex +local keyPrefix = ARGV[11] +-- TTL (seconds) applied to counter lazy-init SETs +local counterTtl = ARGV[12] +-- TTL (seconds) applied to ckVtime on registration +local stateTtl = ARGV[13] -if #expiredMembers == 0 then - return {} -end +-- Fast path: check if we can skip the queue and go directly to worker queue +if enableFastPath == '1' then + local available = redis.call('ZRANGEBYSCORE', queueKey, '-inf', currentTime, 'LIMIT', 0, 1) + if #available == 0 then + local envCurrent = tonumber(redis.call('SCARD', envCurrentConcurrencyKey) or '0') + local envLimit = tonumber(redis.call('GET', envConcurrencyLimitKey) or defaultEnvConcurrencyLimit) + local envBurstFactor = tonumber(redis.call('GET', envConcurrencyLimitBurstFactorKey) or defaultEnvConcurrencyBurstFactor) + local envLimitWithBurst = math.floor(envLimit * envBurstFactor) -local time = redis.call('TIME') -local nowMs = tonumber(time[1]) * 1000 + math.floor(tonumber(time[2]) / 1000) + if envCurrent < envLimitWithBurst then + local queueCurrent = tonumber(redis.call('SCARD', queueCurrentConcurrencyKey) or '0') + local queueLimit = math.min( + tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'), + envLimit + ) -local results = {} + if queueCurrent < queueLimit then + redis.call('SET', messageKey, messageData) + redis.call('SADD', queueCurrentConcurrencyKey, messageId) + redis.call('SADD', envCurrentConcurrencyKey, messageId) + redis.call('RPUSH', workerQueueKey, messageKeyValue) + -- Fast-path skips the CK variant zset entirely; lengthCounter is unchanged. + -- runningCounter is bumped later by dequeueMessageFromKeyTracked when the + -- worker pulls the message from the worker queue. + return 1 + end + end + end +end -for i, member in ipairs(expiredMembers) do - -- Parse member format: "queueKey|runId|orgId" - local pipePos1 = string.find(member, "|", 1, true) - if pipePos1 then - local pipePos2 = string.find(member, "|", pipePos1 + 1, true) - if pipePos2 then - local rawQueueKey = string.sub(member, 1, pipePos1 - 1) - local runId = string.sub(member, pipePos1 + 1, pipePos2 - 1) - local orgId = string.sub(member, pipePos2 + 1) +-- Slow path: normal enqueue +redis.call('SET', messageKey, messageData) - -- Prefix the queue key so it matches the actual Redis keys - local queueKey = keyPrefix .. rawQueueKey +-- Lazy-init lengthCounter from existing ckIndex variants (once per base queue per 24h). +-- The 24h TTL means the counter periodically re-anchors to truth, bounding any drift +-- that accumulated during rolling-deploy overlap windows. +-- Run BEFORE the ZADD so we capture pre-state; the subsequent INCR accounts for the new message. +-- The counter tracks ONLY CK-variant messages — the read path adds ZCARD(base) separately, +-- so the base zset is intentionally excluded here. +if redis.call('EXISTS', lengthCounterKey) == 0 then + local total = 0 + local variants = redis.call('ZRANGE', ckIndexKey, 0, -1) + for _, v in ipairs(variants) do + total = total + tonumber(redis.call('ZCARD', keyPrefix .. v) or '0') + end + redis.call('SET', lengthCounterKey, total, 'EX', counterTtl) +end - -- Remove from TTL set - redis.call('ZREM', ttlQueueKey, member) +-- INCR is gated on ZADD returning 1 (new entry). A duplicate enqueue (same messageId +-- already in the variant zset) returns 0 and must not bump the counter. +local added = redis.call('ZADD', queueKey, messageScore, messageId) +redis.call('ZADD', envQueueKey, messageScore, messageId) +if added == 1 then + redis.call('INCR', lengthCounterKey) +end - -- Construct keys for acknowledging the run from normal queue - -- Extract org from rawQueueKey: {org:orgId}:proj:... - local orgKeyStart = string.find(rawQueueKey, "{org:", 1, true) - local orgKeyEnd = string.find(rawQueueKey, "}", orgKeyStart, true) - local orgFromQueue = string.sub(rawQueueKey, orgKeyStart + 5, orgKeyEnd - 1) +-- Rebalance CK index +local earliest = redis.call('ZRANGE', queueKey, 0, 0, 'WITHSCORES') +if #earliest > 0 then + redis.call('ZADD', ckIndexKey, earliest[2], queueName) +end - local messageKey = keyPrefix .. "{org:" .. orgFromQueue .. "}:message:" .. runId +-- Register this variant in the virtual-time index at the floor. NX means an +-- already-advanced tag is never rewound. +local vfloor = redis.call('GET', ckVtimeFloorKey) or '0' +redis.call('ZADD', ckVtimeKey, 'NX', vfloor, queueName) +redis.call('EXPIRE', ckVtimeKey, stateTtl) +redis.call('EXPIRE', ckVtimeFloorKey, stateTtl) - -- Delete message key - redis.call('DEL', messageKey) +-- Rebalance master queue with ck:* member +local earliestIdx = redis.call('ZRANGE', ckIndexKey, 0, 0, 'WITHSCORES') +if #earliestIdx > 0 then + redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) +end - -- Remove from queue sorted set - redis.call('ZREM', queueKey, runId) +-- Remove old-format entry from master queue (transition cleanup) +redis.call('ZREM', masterQueueKey, queueName) - -- Remove from env queue (derive from rawQueueKey) - -- rawQueueKey format: {org:X}:proj:Y:env:Z:queue:Q[:ck:C] - local envMatch = string.match(rawQueueKey, ":env:([^:]+)") - if envMatch then - local envQueueKey = keyPrefix .. "{org:" .. orgFromQueue .. "}:env:" .. envMatch - redis.call('ZREM', envQueueKey, runId) - end +-- Update the concurrency keys +redis.call('SREM', queueCurrentConcurrencyKey, messageId) +redis.call('SREM', envCurrentConcurrencyKey, messageId) +redis.call('SREM', queueCurrentDequeuedKey, messageId) +redis.call('SREM', envCurrentDequeuedKey, messageId) - -- Remove from concurrency sets - local concurrencyKey = queueKey .. ":currentConcurrency" +return 0 + `, + }); + + // Vtime variant of enqueueMessageWithTtlCkTracked. Same slow-path-only + // registration as enqueueMessageCkVtimeTracked above. + this.redis.defineCommand("enqueueMessageWithTtlCkVtimeTracked", { + numberOfKeys: 18, + lua: ` +local masterQueueKey = KEYS[1] +local queueKey = KEYS[2] +local messageKey = KEYS[3] +local queueCurrentConcurrencyKey = KEYS[4] +local envCurrentConcurrencyKey = KEYS[5] +local queueCurrentDequeuedKey = KEYS[6] +local envCurrentDequeuedKey = KEYS[7] +local envQueueKey = KEYS[8] +local ttlQueueKey = KEYS[9] +local ckIndexKey = KEYS[10] +-- Fast-path keys (KEYS 11-14) +local workerQueueKey = KEYS[11] +local queueConcurrencyLimitKey = KEYS[12] +local envConcurrencyLimitKey = KEYS[13] +local envConcurrencyLimitBurstFactorKey = KEYS[14] +-- Counter keys (KEYS 15-16) +local lengthCounterKey = KEYS[15] +local baseQueueKey = KEYS[16] +-- Virtual-time keys (KEYS 17-18) +local ckVtimeKey = KEYS[17] +local ckVtimeFloorKey = KEYS[18] + +local queueName = ARGV[1] +local messageId = ARGV[2] +local messageData = ARGV[3] +local messageScore = ARGV[4] +local ttlMember = ARGV[5] +local ttlScore = ARGV[6] +local ckWildcardName = ARGV[7] +-- Fast-path args (ARGV 8-12) +local messageKeyValue = ARGV[8] +local defaultEnvConcurrencyLimit = ARGV[9] +local defaultEnvConcurrencyBurstFactor = ARGV[10] +local currentTime = ARGV[11] +local enableFastPath = ARGV[12] +-- keyPrefix for prepending to variant names stored as values in ckIndex +local keyPrefix = ARGV[13] +-- TTL (seconds) applied to counter lazy-init SETs +local counterTtl = ARGV[14] +-- TTL (seconds) applied to ckVtime on registration +local stateTtl = ARGV[15] + +-- Fast path: check if we can skip the queue and go directly to worker queue +if enableFastPath == '1' then + local available = redis.call('ZRANGEBYSCORE', queueKey, '-inf', currentTime, 'LIMIT', 0, 1) + if #available == 0 then + local envCurrent = tonumber(redis.call('SCARD', envCurrentConcurrencyKey) or '0') + local envLimit = tonumber(redis.call('GET', envConcurrencyLimitKey) or defaultEnvConcurrencyLimit) + local envBurstFactor = tonumber(redis.call('GET', envConcurrencyLimitBurstFactorKey) or defaultEnvConcurrencyBurstFactor) + local envLimitWithBurst = math.floor(envLimit * envBurstFactor) + + if envCurrent < envLimitWithBurst then + local queueCurrent = tonumber(redis.call('SCARD', queueCurrentConcurrencyKey) or '0') + local queueLimit = math.min( + tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'), + envLimit + ) + + if queueCurrent < queueLimit then + redis.call('SET', messageKey, messageData) + redis.call('SADD', queueCurrentConcurrencyKey, messageId) + redis.call('SADD', envCurrentConcurrencyKey, messageId) + redis.call('RPUSH', workerQueueKey, messageKeyValue) + return 1 + end + end + end +end + +-- Slow path: normal enqueue +redis.call('SET', messageKey, messageData) + +-- Lazy-init lengthCounter from existing ckIndex variants (once per base queue per 24h). +-- See enqueueMessageCkTracked for the TTL rationale. +if redis.call('EXISTS', lengthCounterKey) == 0 then + local total = 0 + local variants = redis.call('ZRANGE', ckIndexKey, 0, -1) + for _, v in ipairs(variants) do + total = total + tonumber(redis.call('ZCARD', keyPrefix .. v) or '0') + end + redis.call('SET', lengthCounterKey, total, 'EX', counterTtl) +end + +-- INCR is gated on ZADD returning 1 (new entry). +local added = redis.call('ZADD', queueKey, messageScore, messageId) +redis.call('ZADD', envQueueKey, messageScore, messageId) +redis.call('ZADD', ttlQueueKey, ttlScore, ttlMember) +if added == 1 then + redis.call('INCR', lengthCounterKey) +end + +-- Rebalance CK index +local earliest = redis.call('ZRANGE', queueKey, 0, 0, 'WITHSCORES') +if #earliest > 0 then + redis.call('ZADD', ckIndexKey, earliest[2], queueName) +end + +-- Register this variant in the virtual-time index at the floor. NX means an +-- already-advanced tag is never rewound. +local vfloor = redis.call('GET', ckVtimeFloorKey) or '0' +redis.call('ZADD', ckVtimeKey, 'NX', vfloor, queueName) +redis.call('EXPIRE', ckVtimeKey, stateTtl) +redis.call('EXPIRE', ckVtimeFloorKey, stateTtl) + +-- Rebalance master queue with ck:* member +local earliestIdx = redis.call('ZRANGE', ckIndexKey, 0, 0, 'WITHSCORES') +if #earliestIdx > 0 then + redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) +end + +-- Remove old-format entry from master queue (transition cleanup) +redis.call('ZREM', masterQueueKey, queueName) + +-- Update the concurrency keys +redis.call('SREM', queueCurrentConcurrencyKey, messageId) +redis.call('SREM', envCurrentConcurrencyKey, messageId) +redis.call('SREM', queueCurrentDequeuedKey, messageId) +redis.call('SREM', envCurrentDequeuedKey, messageId) + +return 0 + `, + }); + + // Expire TTL runs - atomically removes from TTL set, acknowledges from normal queue, and enqueues to TTL worker + this.redis.defineCommand("expireTtlRuns", { + numberOfKeys: 1, + lua: ` +local ttlQueueKey = KEYS[1] +local keyPrefix = ARGV[1] +local currentTime = tonumber(ARGV[2]) +local batchSize = tonumber(ARGV[3]) +local shardCount = tonumber(ARGV[4]) +local workerQueueKey = ARGV[5] +local workerItemsKey = ARGV[6] +local visibilityTimeoutMs = tonumber(ARGV[7]) + +-- Get expired runs from TTL sorted set (score <= currentTime) +local expiredMembers = redis.call('ZRANGEBYSCORE', ttlQueueKey, '-inf', currentTime, 'LIMIT', 0, batchSize) + +if #expiredMembers == 0 then + return {} +end + +local time = redis.call('TIME') +local nowMs = tonumber(time[1]) * 1000 + math.floor(tonumber(time[2]) / 1000) + +local results = {} + +for i, member in ipairs(expiredMembers) do + -- Parse member format: "queueKey|runId|orgId" + local pipePos1 = string.find(member, "|", 1, true) + if pipePos1 then + local pipePos2 = string.find(member, "|", pipePos1 + 1, true) + if pipePos2 then + local rawQueueKey = string.sub(member, 1, pipePos1 - 1) + local runId = string.sub(member, pipePos1 + 1, pipePos2 - 1) + local orgId = string.sub(member, pipePos2 + 1) + + -- Prefix the queue key so it matches the actual Redis keys + local queueKey = keyPrefix .. rawQueueKey + + -- Remove from TTL set + redis.call('ZREM', ttlQueueKey, member) + + -- Construct keys for acknowledging the run from normal queue + -- Extract org from rawQueueKey: {org:orgId}:proj:... + local orgKeyStart = string.find(rawQueueKey, "{org:", 1, true) + local orgKeyEnd = string.find(rawQueueKey, "}", orgKeyStart, true) + local orgFromQueue = string.sub(rawQueueKey, orgKeyStart + 5, orgKeyEnd - 1) + + local messageKey = keyPrefix .. "{org:" .. orgFromQueue .. "}:message:" .. runId + + -- Delete message key + redis.call('DEL', messageKey) + + -- Remove from queue sorted set + redis.call('ZREM', queueKey, runId) + + -- Remove from env queue (derive from rawQueueKey) + -- rawQueueKey format: {org:X}:proj:Y:env:Z:queue:Q[:ck:C] + local envMatch = string.match(rawQueueKey, ":env:([^:]+)") + if envMatch then + local envQueueKey = keyPrefix .. "{org:" .. orgFromQueue .. "}:env:" .. envMatch + redis.call('ZREM', envQueueKey, runId) + end + + -- Remove from concurrency sets + local concurrencyKey = queueKey .. ":currentConcurrency" local dequeuedKey = queueKey .. ":currentDequeued" redis.call('SREM', concurrencyKey, runId) redis.call('SREM', dequeuedKey, runId) @@ -4281,6 +4708,198 @@ else redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) end +return results + `, + }); + + // Virtual-time (SFQ) variant of dequeueMessagesFromCkQueueTracked. + // Flag-selected via ckVirtualTimeScheduling. Orders concurrency-key variants + // by virtual-time tag (ckVtime ZSET) instead of head timestamp, layered under + // the existing per-variant concurrency gate. Two passes: pass 1 serves in fair + // (lowest-tag) order; pass 2 fills the batch + discovers unregistered variants + // in the existing age order (work conservation, mixed-deploy safety). Only the + // :ckVtime / :ckVtimeFloor keys hold virtual times; ckIndex and the master + // queue keep their timestamp score domain. The per-candidate serve body is a + // verbatim copy of dequeueMessagesFromCkQueueTracked's, with the marked NEW + // lines added (tag advance on serve, ZREM ckVtime on GC). + this.redis.defineCommand("dequeueMessagesFromCkQueueVtimeTracked", { + numberOfKeys: 12, + lua: ` +local ckIndexKey = KEYS[1] +local queueConcurrencyLimitKey = KEYS[2] +local envConcurrencyLimitKey = KEYS[3] +local envConcurrencyLimitBurstFactorKey = KEYS[4] +local envCurrentConcurrencyKey = KEYS[5] +local messageKeyPrefix = KEYS[6] +local envQueueKey = KEYS[7] +local masterQueueKey = KEYS[8] +local ttlQueueKey = KEYS[9] +local lengthCounterKey = KEYS[10] +local ckVtimeKey = KEYS[11] +local ckVtimeFloorKey = KEYS[12] + +local ckWildcardName = ARGV[1] +local currentTime = tonumber(ARGV[2]) +local defaultEnvConcurrencyLimit = ARGV[3] +local defaultEnvConcurrencyBurstFactor = ARGV[4] +local keyPrefix = ARGV[5] +local maxCount = tonumber(ARGV[6] or '1') +local quantum = tonumber(ARGV[7] or '1') +local windowMultiplier = tonumber(ARGV[8] or '3') +local stateTtl = tonumber(ARGV[9] or '86400') + +local function decrLengthCounter() + if tonumber(redis.call('GET', lengthCounterKey) or '0') > 0 then + redis.call('DECR', lengthCounterKey) + end +end + +-- Check env concurrency +local envCurrentConcurrency = tonumber(redis.call('SCARD', envCurrentConcurrencyKey) or '0') +local envConcurrencyLimit = tonumber(redis.call('GET', envConcurrencyLimitKey) or defaultEnvConcurrencyLimit) +local envConcurrencyLimitBurstFactor = tonumber(redis.call('GET', envConcurrencyLimitBurstFactorKey) or defaultEnvConcurrencyBurstFactor) +local envConcurrencyLimitWithBurstFactor = math.floor(envConcurrencyLimit * envConcurrencyLimitBurstFactor) + +if envCurrentConcurrency >= envConcurrencyLimitWithBurstFactor then + return nil +end + +local queueConcurrencyLimit = math.min(tonumber(redis.call('GET', queueConcurrencyLimitKey) or '1000000'), envConcurrencyLimit) + +local envAvailableCapacity = envConcurrencyLimitWithBurstFactor - envCurrentConcurrency +local actualMaxCount = math.min(maxCount, envAvailableCapacity) + +if actualMaxCount <= 0 then + return nil +end + +local window = actualMaxCount * windowMultiplier + +-- Monotonic floor, advanced to the minimum stored virtual-time tag +local floor = tonumber(redis.call('GET', ckVtimeFloorKey) or '0') +local minEntry = redis.call('ZRANGE', ckVtimeKey, 0, 0, 'WITHSCORES') +if #minEntry > 0 then + local minTag = tonumber(minEntry[2]) + if minTag > floor then + floor = minTag + end +end + +local results = {} +local dequeuedCount = 0 +local attempted = {} + +-- Per-candidate serve. Body is dequeueMessagesFromCkQueueTracked's per-candidate +-- block, verbatim, with the marked NEW lines added. +local function tryServe(ckQueueName) + attempted[ckQueueName] = true + local fullQueueKey = keyPrefix .. ckQueueName + + local ckConcurrencyKey = fullQueueKey .. ':currentConcurrency' + local ckCurrentConcurrency = tonumber(redis.call('SCARD', ckConcurrencyKey) or '0') + + if ckCurrentConcurrency < queueConcurrencyLimit then + local messages = redis.call('ZRANGEBYSCORE', fullQueueKey, '-inf', tostring(currentTime), 'WITHSCORES', 'LIMIT', 0, 1) + + if #messages >= 2 then + local messageId = messages[1] + local messageScore = messages[2] + + local messageKey = messageKeyPrefix .. messageId + local messagePayload = redis.call('GET', messageKey) + + if messagePayload then + local messageData = cjson.decode(messagePayload) + local ttlExpiresAt = messageData and messageData.ttlExpiresAt + + if ttlExpiresAt and ttlExpiresAt <= currentTime then + redis.call('ZREM', fullQueueKey, messageId) + redis.call('ZREM', envQueueKey, messageId) + decrLengthCounter() + else + redis.call('ZREM', fullQueueKey, messageId) + redis.call('ZREM', envQueueKey, messageId) + decrLengthCounter() + redis.call('SADD', ckConcurrencyKey, messageId) + redis.call('SADD', envCurrentConcurrencyKey, messageId) + + if ttlQueueKey and ttlQueueKey ~= '' and ttlExpiresAt then + local ttlMember = ckQueueName .. '|' .. messageId .. '|' .. (messageData.orgId or '') + redis.call('ZREM', ttlQueueKey, ttlMember) + end + + table.insert(results, messageId) + table.insert(results, messageScore) + table.insert(results, messagePayload) + + dequeuedCount = dequeuedCount + 1 + + -- NEW: advance this variant's virtual time (weight hook: fixed 1 today) + local weight = 1 + local tag = tonumber(redis.call('ZSCORE', ckVtimeKey, ckQueueName) or floor) + if tag < floor then tag = floor end + redis.call('ZADD', ckVtimeKey, tostring(tag + (quantum / weight)), ckQueueName) + end + else + redis.call('ZREM', fullQueueKey, messageId) + redis.call('ZREM', envQueueKey, messageId) + decrLengthCounter() + end + + local earliest = redis.call('ZRANGE', fullQueueKey, 0, 0, 'WITHSCORES') + if #earliest == 0 then + redis.call('ZREM', ckIndexKey, ckQueueName) + redis.call('ZREM', ckVtimeKey, ckQueueName) -- NEW + else + redis.call('ZADD', ckIndexKey, earliest[2], ckQueueName) + end + else + local any = redis.call('ZRANGE', fullQueueKey, 0, 0, 'WITHSCORES') + if #any == 0 then + redis.call('ZREM', ckIndexKey, ckQueueName) + redis.call('ZREM', ckVtimeKey, ckQueueName) -- NEW + else + redis.call('ZADD', ckIndexKey, any[2], ckQueueName) + end + end + end +end + +-- Pass 1: fair order (lowest virtual start tag first) +local vtimeCandidates = redis.call('ZRANGE', ckVtimeKey, 0, window - 1) +for _, ckQueueName in ipairs(vtimeCandidates) do + if dequeuedCount >= actualMaxCount then break end + tryServe(ckQueueName) +end + +-- Pass 2: fill + discovery in age order (work conservation, mixed-deploy safety). +-- Never runs when pass 1 filled the batch. +if dequeuedCount < actualMaxCount then + -- Clamp to at least 3x so pass 2 never scans fewer index variants than the old command, preserving work conservation regardless of the configured multiplier + local pass2Window = math.max(window, actualMaxCount * 3) + local ckQueues = redis.call('ZRANGEBYSCORE', ckIndexKey, '-inf', tostring(currentTime), 'LIMIT', 0, pass2Window) + for _, ckQueueName in ipairs(ckQueues) do + if dequeuedCount >= actualMaxCount then break end + if not attempted[ckQueueName] then + tryServe(ckQueueName) + end + end +end + +-- NEW: persist floor and refresh TTLs +redis.call('SET', ckVtimeFloorKey, tostring(floor), 'EX', stateTtl) +if redis.call('EXISTS', ckVtimeKey) == 1 then + redis.call('EXPIRE', ckVtimeKey, stateTtl) +end + +-- Rebalance master queue (ckIndex keeps its timestamp domain) +local earliestIdx = redis.call('ZRANGE', ckIndexKey, 0, 0, 'WITHSCORES') +if #earliestIdx == 0 then + redis.call('ZREM', masterQueueKey, ckWildcardName) +else + redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) +end + return results `, }); @@ -4876,6 +5495,110 @@ else redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) end +-- Remove old-format entry from master queue (transition cleanup) +redis.call('ZREM', masterQueueKey, messageQueueName) +`, + }); + + // Vtime variant of nackMessageCkTracked (feature-flagged via + // ckVirtualTimeScheduling.enabled). Identical script body, plus registration + // of the variant into the :ckVtime ZSET at the floor (NX), so a GC'd variant + // that a nack revives rejoins the fair order. + this.redis.defineCommand("nackMessageCkVtimeTracked", { + numberOfKeys: 13, + lua: ` +-- Keys: +local masterQueueKey = KEYS[1] +local messageKey = KEYS[2] +local messageQueueKey = KEYS[3] +local queueCurrentConcurrencyKey = KEYS[4] +local envCurrentConcurrencyKey = KEYS[5] +local queueCurrentDequeuedKey = KEYS[6] +local envCurrentDequeuedKey = KEYS[7] +local envQueueKey = KEYS[8] +local ckIndexKey = KEYS[9] +local lengthCounterKey = KEYS[10] +local runningCounterKey = KEYS[11] +-- Virtual-time keys (KEYS 12-13) +local ckVtimeKey = KEYS[12] +local ckVtimeFloorKey = KEYS[13] + +-- Args: +local messageId = ARGV[1] +local messageQueueName = ARGV[2] +local messageData = ARGV[3] +local messageScore = tonumber(ARGV[4]) +local ckWildcardName = ARGV[5] +-- keyPrefix for prepending to variant names stored as values in ckIndex (lazy-init only) +local keyPrefix = ARGV[6] +-- TTL (seconds) applied to counter lazy-init SETs +local counterTtl = ARGV[7] +-- TTL (seconds) applied to ckVtime on registration +local stateTtl = ARGV[8] + +local function decrFloored(key) + if tonumber(redis.call('GET', key) or '0') > 0 then + redis.call('DECR', key) + end +end + +-- Update the message data +redis.call('SET', messageKey, messageData) + +-- Update the concurrency keys. nack only DECRs runningCounter, never INCRs it, +-- so we skip the eager lazy-init here (unlike releaseConcurrencyTracked, which +-- mirrors the same DECR pattern with init). A post-TTL nack's floored DECR +-- no-ops; the next dequeueMessageFromKeyTracked reseeds from current state. +redis.call('SREM', queueCurrentConcurrencyKey, messageId) +redis.call('SREM', envCurrentConcurrencyKey, messageId) +local removedFromDequeued = redis.call('SREM', queueCurrentDequeuedKey, messageId) +redis.call('SREM', envCurrentDequeuedKey, messageId) +if removedFromDequeued == 1 then + decrFloored(runningCounterKey) +end + +-- Lazy-init lengthCounter if missing (e.g. expired via 24h TTL). nack re-queues a +-- message, which means lengthCounter must be present before we INCR. Without this, +-- a nack after counter expiry would create the counter at 1 and stay drifted until +-- next reset. +if redis.call('EXISTS', lengthCounterKey) == 0 then + local total = 0 + local variants = redis.call('ZRANGE', ckIndexKey, 0, -1) + for _, v in ipairs(variants) do + total = total + tonumber(redis.call('ZCARD', keyPrefix .. v) or '0') + end + redis.call('SET', lengthCounterKey, total, 'EX', counterTtl) +end + +-- Enqueue the message back into the CK-specific queue. INCR lengthCounter only if +-- it's a new entry (ZADD returns 1). +local added = redis.call('ZADD', messageQueueKey, messageScore, messageId) +redis.call('ZADD', envQueueKey, messageScore, messageId) +if added == 1 then + redis.call('INCR', lengthCounterKey) +end + +-- Rebalance CK index +local earliest = redis.call('ZRANGE', messageQueueKey, 0, 0, 'WITHSCORES') +if #earliest > 0 then + redis.call('ZADD', ckIndexKey, earliest[2], messageQueueName) +end + +-- Register this variant in the virtual-time index at the floor. NX means an +-- already-advanced tag is never rewound. +local vfloor = redis.call('GET', ckVtimeFloorKey) or '0' +redis.call('ZADD', ckVtimeKey, 'NX', vfloor, messageQueueName) +redis.call('EXPIRE', ckVtimeKey, stateTtl) +redis.call('EXPIRE', ckVtimeFloorKey, stateTtl) + +-- Rebalance master queue with ck:* member +local earliestIdx = redis.call('ZRANGE', ckIndexKey, 0, 0, 'WITHSCORES') +if #earliestIdx == 0 then + redis.call('ZREM', masterQueueKey, ckWildcardName) +else + redis.call('ZADD', masterQueueKey, earliestIdx[2], ckWildcardName) +end + -- Remove old-format entry from master queue (transition cleanup) redis.call('ZREM', masterQueueKey, messageQueueName) `, @@ -5588,6 +6311,77 @@ declare module "@internal/redis" { callback?: Callback ): Result; + enqueueMessageCkVtimeTracked( + masterQueueKey: string, + queue: string, + messageKey: string, + queueCurrentConcurrencyKey: string, + envCurrentConcurrencyKey: string, + queueCurrentDequeuedKey: string, + envCurrentDequeuedKey: string, + envQueueKey: string, + ckIndexKey: string, + workerQueueKey: string, + queueConcurrencyLimitKey: string, + envConcurrencyLimitKey: string, + envConcurrencyLimitBurstFactorKey: string, + lengthCounterKey: string, + baseQueueKey: string, + ckVtimeKey: string, + ckVtimeFloorKey: string, + queueName: string, + messageId: string, + messageData: string, + messageScore: string, + ckWildcardName: string, + messageKeyValue: string, + defaultEnvConcurrencyLimit: string, + defaultEnvConcurrencyBurstFactor: string, + currentTime: string, + enableFastPath: string, + keyPrefix: string, + counterTtl: string, + stateTtl: string, + callback?: Callback + ): Result; + + enqueueMessageWithTtlCkVtimeTracked( + masterQueueKey: string, + queue: string, + messageKey: string, + queueCurrentConcurrencyKey: string, + envCurrentConcurrencyKey: string, + queueCurrentDequeuedKey: string, + envCurrentDequeuedKey: string, + envQueueKey: string, + ttlQueueKey: string, + ckIndexKey: string, + workerQueueKey: string, + queueConcurrencyLimitKey: string, + envConcurrencyLimitKey: string, + envConcurrencyLimitBurstFactorKey: string, + lengthCounterKey: string, + baseQueueKey: string, + ckVtimeKey: string, + ckVtimeFloorKey: string, + queueName: string, + messageId: string, + messageData: string, + messageScore: string, + ttlMember: string, + ttlScore: string, + ckWildcardName: string, + messageKeyValue: string, + defaultEnvConcurrencyLimit: string, + defaultEnvConcurrencyBurstFactor: string, + currentTime: string, + enableFastPath: string, + keyPrefix: string, + counterTtl: string, + stateTtl: string, + callback?: Callback + ): Result; + dequeueMessagesFromCkQueueTracked( ckIndexKey: string, queueConcurrencyLimitKey: string, @@ -5608,6 +6402,31 @@ declare module "@internal/redis" { callback?: Callback ): Result; + dequeueMessagesFromCkQueueVtimeTracked( + ckIndexKey: string, + queueConcurrencyLimitKey: string, + envConcurrencyLimitKey: string, + envConcurrencyLimitBurstFactorKey: string, + envCurrentConcurrencyKey: string, + messageKeyPrefix: string, + envQueueKey: string, + masterQueueKey: string, + ttlQueueKey: string, + lengthCounterKey: string, + ckVtimeKey: string, + ckVtimeFloorKey: string, + ckWildcardName: string, + currentTime: string, + defaultEnvConcurrencyLimit: string, + defaultEnvConcurrencyBurstFactor: string, + keyPrefix: string, + maxCount: string, + quantum: string, + windowMultiplier: string, + stateTtlSeconds: string, + callback?: Callback + ): Result; + dequeueMessageFromKeyTracked( messageKey: string, keyPrefix: string, @@ -5658,6 +6477,31 @@ declare module "@internal/redis" { callback?: Callback ): Result; + nackMessageCkVtimeTracked( + masterQueueKey: string, + messageKey: string, + messageQueue: string, + queueCurrentConcurrencyKey: string, + envCurrentConcurrencyKey: string, + queueCurrentDequeuedKey: string, + envCurrentDequeuedKey: string, + envQueueKey: string, + ckIndexKey: string, + lengthCounterKey: string, + runningCounterKey: string, + ckVtimeKey: string, + ckVtimeFloorKey: string, + messageId: string, + messageQueueName: string, + messageData: string, + messageScore: string, + ckWildcardName: string, + keyPrefix: string, + counterTtl: string, + stateTtl: string, + callback?: Callback + ): Result; + moveToDeadLetterQueueCkTracked( masterQueueKey: string, messageKey: string, diff --git a/internal-packages/run-engine/src/run-queue/keyProducer.ts b/internal-packages/run-engine/src/run-queue/keyProducer.ts index 3be79a4fabc..4fbe28818be 100644 --- a/internal-packages/run-engine/src/run-queue/keyProducer.ts +++ b/internal-packages/run-engine/src/run-queue/keyProducer.ts @@ -22,6 +22,8 @@ const constants = { MASTER_QUEUE_PART: "masterQueue", WORKER_QUEUE_PART: "workerQueue", CK_INDEX_PART: "ckIndex", + CK_VTIME_PART: "ckVtime", + CK_VTIME_FLOOR_PART: "ckVtimeFloor", LENGTH_COUNTER_PART: "lengthCounter", RUNNING_COUNTER_PART: "runningCounter", } as const; @@ -317,6 +319,14 @@ export class RunQueueFullKeyProducer implements RunQueueKeyProducer { return `${baseQueue}:${constants.CK_INDEX_PART}`; } + ckVtimeKeyFromQueue(queue: string): string { + return `${this.baseQueueKeyFromQueue(queue)}:${constants.CK_VTIME_PART}`; + } + + ckVtimeFloorKeyFromQueue(queue: string): string { + return `${this.baseQueueKeyFromQueue(queue)}:${constants.CK_VTIME_FLOOR_PART}`; + } + baseQueueKeyFromQueue(queue: string): string { return queue.replace(/:ck:.+$/, ""); } diff --git a/internal-packages/run-engine/src/run-queue/tests/ckVtime.test.ts b/internal-packages/run-engine/src/run-queue/tests/ckVtime.test.ts new file mode 100644 index 00000000000..98b0ac73d17 --- /dev/null +++ b/internal-packages/run-engine/src/run-queue/tests/ckVtime.test.ts @@ -0,0 +1,1061 @@ +import { redisTest } from "@internal/testcontainers"; +import { trace } from "@internal/tracing"; +import { Logger } from "@trigger.dev/core/logger"; +import { Decimal } from "@trigger.dev/database"; +import { describe } from "node:test"; +import { FairQueueSelectionStrategy } from "../fairQueueSelectionStrategy.js"; +import { RunQueue } from "../index.js"; +import { RunQueueFullKeyProducer } from "../keyProducer.js"; +import type { InputPayload } from "../types.js"; + +const testOptions = { + name: "rq", + tracer: trace.getTracer("rq"), + workers: 1, + defaultEnvConcurrency: 25, + logger: new Logger("RunQueue", "warn"), + retryOptions: { + maxAttempts: 5, + factor: 1.1, + minTimeoutInMs: 100, + maxTimeoutInMs: 1_000, + randomize: true, + }, + keys: new RunQueueFullKeyProducer(), +}; + +const authenticatedEnvDev = { + id: "e1234", + type: "DEVELOPMENT" as const, + maximumConcurrencyLimit: 10, + concurrencyLimitBurstFactor: new Decimal(2.0), + project: { id: "p1234" }, + organization: { id: "o1234" }, +}; + +type VtimeOverrides = { + enabled?: boolean; + quantum?: number; + scanWindowMultiplier?: number; + stateTtlSeconds?: number; +}; + +// vtime: overrides merged into an enabled ckVirtualTimeScheduling option, or +// null to omit the option entirely (flag off, the production default). +function createQueue(redisContainer: any, vtime: VtimeOverrides | null = {}) { + return new RunQueue({ + ...testOptions, + // These tests drive every op themselves (testDequeueFromMasterQueue + skipDequeueProcessing), + // so the autonomous master-queue consumers and background worker must not race them. + masterQueueConsumersDisabled: true, + workerOptions: { disabled: true }, + ...(vtime === null + ? {} + : { + ckVirtualTimeScheduling: { + enabled: true, + ...vtime, + }, + }), + queueSelectionStrategy: new FairQueueSelectionStrategy({ + redis: { + keyPrefix: "runqueue:test:", + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + keys: testOptions.keys, + }), + redis: { + keyPrefix: "runqueue:test:", + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + }); +} + +function makeMessage(overrides: Partial = {}): InputPayload { + return { + runId: "r1", + taskIdentifier: "task/my-task", + orgId: "o1234", + projectId: "p1234", + environmentId: "e1234", + environmentType: "DEVELOPMENT", + queue: "task/my-task", + timestamp: Date.now(), + attempt: 0, + ...overrides, + }; +} + +// The ckVtime/ckIndex member for a variant is the fully-qualified variant queue +// key (org:proj:env:queue:...:ck:), which is exactly what queueKey() produces. +function variantName(ck: string): string { + return testOptions.keys.queueKey(authenticatedEnvDev, "task/my-task", ck); +} + +const QUEUE = "task/my-task"; + +vi.setConfig({ testTimeout: 60_000 }); + +describe("CK virtual-time (SFQ) dequeue", () => { + redisTest("vtime order beats head-timestamp order", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // 30 old messages on heavy, timestamps t0..t0+29 + for (let i = 0; i < 30; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `h${i}`, concurrencyKey: "heavy", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + // 3 much newer messages on light + for (let i = 0; i < 3; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `l${i}`, concurrencyKey: "light", timestamp: t0 + 1000 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const heavyVariant = variantName("heavy"); + const lightVariant = variantName("light"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavyVariant); + + // Explicit seed is redundant now that enqueue registers variants at the + // floor itself; kept as a belt-and-braces fixture. + await queue.redis.zadd(ckVtimeKey, 0, heavyVariant, 0, lightVariant); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + const lightServedInCall: number[] = []; + const lightSeen = new Set(); + + for (let call = 0; call < 3; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + + // At most one message per variant per call. + const heavyCount = messages.filter((m) => m.message.concurrencyKey === "heavy").length; + const lightCount = messages.filter((m) => m.message.concurrencyKey === "light").length; + expect(heavyCount).toBeLessThanOrEqual(1); + expect(lightCount).toBeLessThanOrEqual(1); + + for (const m of messages) { + if (m.message.concurrencyKey === "light" && !lightSeen.has(m.messageId)) { + lightSeen.add(m.messageId); + lightServedInCall.push(call); + } + // ack to free concurrency between calls + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + // All 3 light messages served within the first 3 calls (age order alone + // would have drained the 30 heavy messages first). + expect(lightSeen.size).toBe(3); + } finally { + await queue.quit(); + } + }); + + redisTest( + "vtime order wins over older head when maxCount forces a choice", + async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // Old messages on heavy: age order strictly favours heavy. + for (let i = 0; i < 5; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `h${i}`, concurrencyKey: "heavy", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + // Newer messages on light (two, so light isn't GC'd after its serve). + for (let i = 0; i < 2; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `l${i}`, + concurrencyKey: "light", + timestamp: t0 + 1000, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const heavyVariant = variantName("heavy"); + const lightVariant = variantName("light"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(heavyVariant); + + // Seed vtime the OPPOSITE way to age order: heavy has the HIGH tag, + // light the LOW tag. + await queue.redis.zadd(ckVtimeKey, 10, heavyVariant, 0, lightVariant); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // maxCount 1 with two ready variants: ordering alone decides who is + // served. Age order (the old command) would pick heavy's older head; + // vtime rank must pick light's lower tag. + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 1); + + expect(messages.length).toBe(1); + expect(messages[0]!.message.concurrencyKey).toBe("light"); + + // Light's tag advanced by the quantum (=1); heavy's is untouched. + const lightTag = Number(await queue.redis.zscore(ckVtimeKey, lightVariant)); + const heavyTag = Number(await queue.redis.zscore(ckVtimeKey, heavyVariant)); + expect(lightTag).toBe(1); + expect(heavyTag).toBe(10); + } finally { + await queue.quit(); + } + } + ); + + redisTest("tags advance per serve within one batched call", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + const cks = ["a", "b", "c", "d", "e"]; + + // Two messages per variant so a single serve doesn't drain (and GC) it, + // letting us observe the advanced tag afterwards. + for (const ck of cks) { + for (let i = 0; i < 2; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-${ck}-${i}`, concurrencyKey: ck, timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const seedArgs: (string | number)[] = []; + for (const ck of cks) { + seedArgs.push(0, variantName(ck)); + } + await queue.redis.zadd(ckVtimeKey, ...(seedArgs as [number, string])); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 5); + + expect(messages.length).toBe(5); + + for (const ck of cks) { + const score = await queue.redis.zscore(ckVtimeKey, variantName(ck)); + expect(Number(score)).toBe(1); + } + } finally { + await queue.quit(); + } + }); + + redisTest("floor is monotonic and read-repairs", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + const cks = ["a", "b"]; + + // enough messages per variant to keep serving for many calls + for (const ck of cks) { + for (let i = 0; i < 25; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-${ck}-${i}`, concurrencyKey: ck, timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("a")); + await queue.redis.zadd(ckVtimeKey, 0, variantName("a"), 0, variantName("b")); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + let prevFloor = 0; + for (let call = 0; call < 20; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floor).toBeGreaterThanOrEqual(prevFloor); + prevFloor = floor; + } + + // Final settle: gate both variants (limit 1 + an occupied slot) so nothing + // is served (no advance), and the floor read-repairs up to the current min tag. + await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, QUEUE, 1); + for (const ck of cks) { + await queue.redis.sadd( + testOptions.keys.queueCurrentConcurrencyKeyFromQueue(variantName(ck)), + "occupant" + ); + } + await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + + const floorAfter = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floorAfter).toBeGreaterThanOrEqual(prevFloor); + + const minEntry = await queue.redis.zrange(ckVtimeKey, 0, 0, "WITHSCORES"); + const minTag = Number(minEntry[1]); + expect(floorAfter).toBe(minTag); + expect(floorAfter).toBeGreaterThan(10); + } finally { + await queue.quit(); + } + }); + + redisTest( + "new key initialises at the floor, not zero and not behind the backlog", + async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + const cks = ["a", "b"]; + + for (const ck of cks) { + for (let i = 0; i < 25; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${ck}-${i}`, + concurrencyKey: ck, + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("a")); + // No direct ZADD seeding: the enqueues above register a and b at the + // initial floor (0) themselves. + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // Drive tags up to ~20. + for (let call = 0; call < 20; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floor).toBeGreaterThan(10); + + // A fresh variant registers itself at the current floor via the + // enqueue-time registration (ZADD NX in the enqueue script). + const freshVariant = variantName("fresh"); + // two messages so the fresh variant isn't GC'd on its first serve + for (let i = 0; i < 2; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-fresh-${i}`, + concurrencyKey: "fresh", + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + const served = messages.some((m) => m.message.concurrencyKey === "fresh"); + expect(served).toBe(true); + + const freshTag = Number(await queue.redis.zscore(ckVtimeKey, freshVariant)); + // initialised at the floor and advanced by quantum (=1), not stuck at 1 + expect(freshTag).toBe(floor + 1); + } finally { + await queue.quit(); + } + } + ); + + // H1 regression: the floor key must not be allowed to expire while ckVtime + // survives. Before the fix, only the dequeue command refreshed the floor + // key's TTL, so a dequeue-quiescent + enqueue-active base queue let the floor + // key expire underneath a live ckVtime; a brand-new variant then read a + // missing floor as 0 and jumped ahead of the whole established backlog. The + // enqueue/nack registration paths now refresh the floor key TTL too. + redisTest( + "enqueue refreshes the floor key TTL and a new variant registers at the current floor", + async ({ redisContainer }) => { + const stateTtlSeconds = 3600; + const queue = createQueue(redisContainer, { stateTtlSeconds }); + try { + const t0 = Date.now() - 100_000; + const cks = ["a", "b"]; + + for (const ck of cks) { + for (let i = 0; i < 25; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${ck}-${i}`, + concurrencyKey: ck, + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("a")); + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // Drive tags and the floor above 0 with a run of serves. + for (let call = 0; call < 20; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floor).toBeGreaterThan(10); + expect(await queue.redis.exists(ckVtimeKey)).toBe(1); + + // Simulate the floor key's TTL decaying toward expiry while dequeues are + // quiescent. Without the fix, only a dequeue would ever bump it back. + await queue.redis.pexpire(ckVtimeFloorKey, 2_000); + + // WITHOUT dequeuing, enqueue several more messages on an existing + // variant. The enqueue registration path must refresh the floor key TTL. + for (let i = 0; i < 5; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-a-more-${i}`, + concurrencyKey: "a", + timestamp: t0 + 500 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + // The floor key TTL was pushed back up to (about) stateTtl, well above + // the 2s decay we forced. + const floorPttl = await queue.redis.pttl(ckVtimeFloorKey); + expect(floorPttl).toBeGreaterThan(2_000); + expect(floorPttl).toBeLessThanOrEqual(stateTtlSeconds * 1000); + + // Enqueue-only activity does not move the floor value itself. + const floorAfter = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floorAfter).toBe(floor); + + // A brand-new variant enqueued now registers at the CURRENT floor, so it + // cannot leapfrog the established backlog back to 0. + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-fresh", concurrencyKey: "fresh", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + const freshTag = Number(await queue.redis.zscore(ckVtimeKey, variantName("fresh"))); + expect(freshTag).toBe(floor); + } finally { + await queue.quit(); + } + } + ); + + redisTest("no service, no advance", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // ck:a has messages but its concurrency slot will be occupied + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-a", concurrencyKey: "a", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + for (const ck of ["b", "c"]) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-${ck}`, concurrencyKey: ck, timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + await queue.redis.zadd( + ckVtimeKey, + 0, + variantName("a"), + 0, + variantName("b"), + 0, + variantName("c") + ); + + // base queue concurrency limit of 1 + await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, QUEUE, 1); + + // occupy ck:a's single slot (equivalent to a prior dequeue-without-ack) + await queue.redis.sadd( + testOptions.keys.queueCurrentConcurrencyKeyFromQueue(variantName("a")), + "occupant" + ); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + + const servedCks = messages.map((m) => m.message.concurrencyKey); + expect(servedCks).not.toContain("a"); + expect(servedCks).toContain("b"); + expect(servedCks).toContain("c"); + + // ck:a's tag is unchanged (never served) + const aTag = Number(await queue.redis.zscore(ckVtimeKey, variantName("a"))); + expect(aTag).toBe(0); + } finally { + await queue.quit(); + } + }); + + redisTest( + "GC on empty variant removes it from ckIndex and ckVtime", + async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-a", concurrencyKey: "a", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + // second variant so ckVtime/ckIndex don't fully disappear, keeping the test focused + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-b", concurrencyKey: "b", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + + const aVariant = variantName("a"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(aVariant); + const ckIndexKey = testOptions.keys.ckIndexKeyFromQueue(aVariant); + await queue.redis.zadd(ckVtimeKey, 0, aVariant, 0, variantName("b")); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + expect(messages.some((m) => m.message.concurrencyKey === "a")).toBe(true); + + const inVtime = await queue.redis.zscore(ckVtimeKey, aVariant); + const inIndex = await queue.redis.zscore(ckIndexKey, aVariant); + expect(inVtime).toBeNull(); + expect(inIndex).toBeNull(); + } finally { + await queue.quit(); + } + } + ); + + redisTest("TTL is set and refreshed on ckVtime and ckVtimeFloor", async ({ redisContainer }) => { + const stateTtlSeconds = 3600; + const queue = createQueue(redisContainer, { stateTtlSeconds }); + try { + const t0 = Date.now() - 100_000; + + // two messages on one variant so ckVtime survives (not GC'd) after a serve + for (let i = 0; i < 2; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-a-${i}`, concurrencyKey: "a", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const aVariant = variantName("a"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(aVariant); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(aVariant); + await queue.redis.zadd(ckVtimeKey, 0, aVariant); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 1); + + const vtimeTtl = await queue.redis.pttl(ckVtimeKey); + const floorTtl = await queue.redis.pttl(ckVtimeFloorKey); + + expect(vtimeTtl).toBeGreaterThan(0); + expect(vtimeTtl).toBeLessThanOrEqual(stateTtlSeconds * 1000); + expect(floorTtl).toBeGreaterThan(0); + expect(floorTtl).toBeLessThanOrEqual(stateTtlSeconds * 1000); + } finally { + await queue.quit(); + } + }); + + redisTest( + "pass 2 fill serves unregistered variants and registers them", + async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // Enqueue on a variant but do NOT register it in ckVtime (simulating an + // enqueue from old code that predates enqueue-time registration, e.g. + // during a rolling deploy). Two messages so the variant survives its + // first serve and we can observe it was registered. + for (let i = 0; i < 2; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-a-${i}`, concurrencyKey: "a", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const aVariant = variantName("a"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(aVariant); + // ensure no ckVtime entry exists for it + await queue.redis.zrem(ckVtimeKey, aVariant); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + + expect(messages.some((m) => m.message.concurrencyKey === "a")).toBe(true); + + const tag = await queue.redis.zscore(ckVtimeKey, aVariant); + expect(tag).not.toBeNull(); + } finally { + await queue.quit(); + } + } + ); + + redisTest("future-scheduled variants are skipped without advance", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // a normal ready variant so the :ck:* wildcard is selected from the master queue + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-now", concurrencyKey: "now", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + // a future-scheduled variant + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: "r-future", + concurrencyKey: "future", + timestamp: Date.now() + 60_000, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("now")); + const futureVariant = variantName("future"); + await queue.redis.zadd(ckVtimeKey, 0, variantName("now"), 5, futureVariant); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + + expect(messages.some((m) => m.message.concurrencyKey === "future")).toBe(false); + + const futureTag = Number(await queue.redis.zscore(ckVtimeKey, futureVariant)); + expect(futureTag).toBe(5); + } finally { + await queue.quit(); + } + }); + + redisTest( + "enqueue registers the variant at the current floor with NX", + async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // two variants with enough messages that the drive loop never drains them + for (const ck of ["a", "b"]) { + for (let i = 0; i < 10; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${ck}-${i}`, + concurrencyKey: ck, + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("a")); + + // enqueue registered both variants at the initial floor (0), before any dequeue + expect(Number(await queue.redis.zscore(ckVtimeKey, variantName("a")))).toBe(0); + expect(Number(await queue.redis.zscore(ckVtimeKey, variantName("b")))).toBe(0); + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // drive the floor up to ~5 via serves + for (let call = 0; call < 8; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floor).toBeGreaterThanOrEqual(5); + + // a fresh key enqueued now lands exactly at the floor (no dequeue in between) + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-fresh-0", concurrencyKey: "fresh", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + expect(Number(await queue.redis.zscore(ckVtimeKey, variantName("fresh")))).toBe(floor); + + // NX: enqueueing on a key whose tag is already 9 never rewinds it + await queue.redis.zadd(ckVtimeKey, 9, variantName("nine")); + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-nine-0", concurrencyKey: "nine", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + expect(Number(await queue.redis.zscore(ckVtimeKey, variantName("nine")))).toBe(9); + } finally { + await queue.quit(); + } + } + ); + + redisTest("fast path leaves vtime state untouched", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const aVariant = variantName("a"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(aVariant); + + // empty variant + free capacity: the fast path fires and skips the + // variant zset entirely + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-fast", concurrencyKey: "a" }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + enableFastPath: true, + }); + + // fast-path proof: nothing landed in the variant zset.. + expect(await queue.redis.zcard(aVariant)).toBe(0); + // ..and no vtime registration happened + expect(await queue.redis.zscore(ckVtimeKey, aVariant)).toBeNull(); + + // saturate capacity so the next enqueue takes the slow path + await queue.updateQueueConcurrencyLimits(authenticatedEnvDev, QUEUE, 1); + await queue.redis.sadd( + testOptions.keys.queueCurrentConcurrencyKeyFromQueue(aVariant), + "occupant" + ); + + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-slow", concurrencyKey: "a" }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + enableFastPath: true, + }); + + // slow path taken and the variant is registered + expect(await queue.redis.zcard(aVariant)).toBe(1); + expect(await queue.redis.zscore(ckVtimeKey, aVariant)).not.toBeNull(); + } finally { + await queue.quit(); + } + }); + + redisTest("nack re-registers a GC'd variant", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const t0 = Date.now() - 100_000; + + // one message on ck:a, plenty on ck:b so serves keep flowing and the floor rises + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: "r-a-0", concurrencyKey: "a", timestamp: t0 }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + for (let i = 0; i < 10; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-b-${i}`, concurrencyKey: "b", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const aVariant = variantName("a"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(aVariant); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(aVariant); + const ckIndexKey = testOptions.keys.ckIndexKeyFromQueue(aVariant); + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // first dequeue serves a's only message: a is drained and GC'd from both indexes + let aMessageId: string | undefined; + const first = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 2); + for (const m of first) { + if (m.message.concurrencyKey === "a") { + aMessageId = m.messageId; + } else { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + expect(aMessageId).toBeDefined(); + expect(await queue.redis.zscore(ckVtimeKey, aVariant)).toBeNull(); + expect(await queue.redis.zscore(ckIndexKey, aVariant)).toBeNull(); + + // drive the floor up via b serves + for (let call = 0; call < 6; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 1); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(floor).toBeGreaterThan(0); + + // nack with an immediate retry score so the revived message is servable now + await queue.nackMessage({ + orgId: authenticatedEnvDev.organization.id, + messageId: aMessageId!, + retryAt: Date.now(), + skipDequeueProcessing: true, + }); + + // the variant is back in ckIndex AND in ckVtime at the floor + expect(await queue.redis.zscore(ckIndexKey, aVariant)).not.toBeNull(); + expect(Number(await queue.redis.zscore(ckVtimeKey, aVariant))).toBe(floor); + + // and a subsequent dequeue serves it + const after = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 10); + expect(after.some((m) => m.messageId === aMessageId)).toBe(true); + } finally { + await queue.quit(); + } + }); + + redisTest("ckVtime membership tracks ckIndex membership", async ({ redisContainer }) => { + const queue = createQueue(redisContainer); + try { + const cks = ["a", "b", "c", "d", "e", "f", "g", "h"]; + const ckIndexKey = testOptions.keys.ckIndexKeyFromQueue(variantName("a")); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // deterministic LCG so failures reproduce + let seed = 123456789; + const rand = () => { + seed = (seed * 1103515245 + 12345) % 2147483648; + return seed / 2147483648; + }; + const pick = (n: number) => Math.floor(rand() * n); + + const inFlight: string[] = []; + let nextRun = 0; + + for (let step = 0; step < 200; step++) { + const op = pick(4); + let opName = "noop"; + + if (op === 0) { + opName = "enqueue"; + const ck = cks[pick(cks.length)]!; + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r${nextRun++}`, + concurrencyKey: ck, + timestamp: Date.now() - 100_000, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } else if (op === 1) { + opName = "dequeue"; + const messages = await queue.testDequeueFromMasterQueue( + shard, + authenticatedEnvDev.id, + 1 + pick(4) + ); + for (const m of messages) { + inFlight.push(m.messageId); + } + } else if (op === 2 && inFlight.length > 0) { + opName = "ack"; + const [id] = inFlight.splice(pick(inFlight.length), 1); + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, id!, { + skipDequeueProcessing: true, + }); + } else if (op === 3 && inFlight.length > 0) { + opName = "nack"; + const [id] = inFlight.splice(pick(inFlight.length), 1); + await queue.nackMessage({ + orgId: authenticatedEnvDev.organization.id, + messageId: id!, + retryAt: Date.now(), + skipDequeueProcessing: true, + }); + } + + // closure invariant: every ckIndex member is a ckVtime member. The + // converse may transiently not hold (stale ckVtime entries GC on scan). + const members = await queue.redis.zrange(ckIndexKey, 0, -1); + for (const member of members) { + const tag = await queue.redis.zscore(ckVtimeKey, member); + expect( + tag, + `step ${step} (${opName}): ${member} in ckIndex but not ckVtime` + ).not.toBeNull(); + } + } + } finally { + await queue.quit(); + } + }); + + redisTest( + "flag off creates no vtime keys and matches head-timestamp order", + async ({ redisContainer }) => { + // ckVirtualTimeScheduling ABSENT: the off path calls the pre-existing + // command names (enqueueMessage*CkTracked, dequeueMessagesFromCkQueueTracked, + // nackMessageCkTracked) whose defineCommand script text this feature never + // edited, so the stronger same-script-SHA guarantee holds by construction. + // What a test CAN observe is asserted here: no vtime state is ever created, + // and the dequeue order is head-timestamp (age) order, matching the + // pre-existing ckIndex.test.ts expectation. + const queue = createQueue(redisContainer, null); + try { + const t0 = Date.now() - 100_000; + + // 3 variants with distinct head ages: old < mid < new, 3 messages each. + const heads: Record = { + old: t0, + mid: t0 + 10_000, + new: t0 + 20_000, + }; + for (const [ck, head] of Object.entries(heads)) { + for (let i = 0; i < 3; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${ck}-${i}`, + concurrencyKey: ck, + timestamp: head + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // The off-path command serves at most one message per variant per call, + // visiting variants in ckIndex (head-timestamp) order: oldest head first. + const first = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 5); + expect(first.map((m) => m.message.concurrencyKey)).toEqual(["old", "mid", "new"]); + + // nack old's head (immediate retry), ack the rest + const nackedId = first[0]!.messageId; + await queue.nackMessage({ + orgId: authenticatedEnvDev.organization.id, + messageId: nackedId, + retryAt: Date.now(), + skipDequeueProcessing: true, + }); + for (const m of first.slice(1)) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + + // Two more batched calls drain the original heads in age order each time. + for (let call = 0; call < 2; call++) { + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 5); + expect(messages.map((m) => m.message.concurrencyKey)).toEqual(["old", "mid", "new"]); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + // Only the nacked message remains; it is re-served. + const last = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 5); + expect(last.length).toBe(1); + expect(last[0]!.messageId).toBe(nackedId); + expect(last[0]!.message.concurrencyKey).toBe("old"); + + // After the whole mixed sequence (enqueues, batched dequeues, a nack, + // acks, one message still in flight so the keyspace is non-empty) no + // vtime state exists at all: no :ckVtime, no :ckVtimeFloor. The KEYS + // scan is safe here because redisTest runs flushall before each test, + // so the DB only holds this test's keys. + const allKeys = await queue.redis.keys("*"); + expect(allKeys.length).toBeGreaterThan(0); + expect(allKeys.filter((k) => k.includes("ckVtime"))).toEqual([]); + + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, nackedId, { + skipDequeueProcessing: true, + }); + } finally { + await queue.quit(); + } + } + ); +}); diff --git a/internal-packages/run-engine/src/run-queue/tests/ckVtimeConcurrency.test.ts b/internal-packages/run-engine/src/run-queue/tests/ckVtimeConcurrency.test.ts new file mode 100644 index 00000000000..9edbd772839 --- /dev/null +++ b/internal-packages/run-engine/src/run-queue/tests/ckVtimeConcurrency.test.ts @@ -0,0 +1,375 @@ +import { createRedisClient } from "@internal/redis"; +import { redisTest } from "@internal/testcontainers"; +import { trace } from "@internal/tracing"; +import { Logger } from "@trigger.dev/core/logger"; +import { Decimal } from "@trigger.dev/database"; +import { setTimeout as sleep } from "node:timers/promises"; +import { describe } from "node:test"; +import { FairQueueSelectionStrategy } from "../fairQueueSelectionStrategy.js"; +import { RunQueue } from "../index.js"; +import { RunQueueFullKeyProducer } from "../keyProducer.js"; +import type { InputPayload } from "../types.js"; + +// Multi-consumer / multi-shard correctness for CK virtual-time scheduling, plus +// an op-count budget pinning the per-dequeue overhead of the vtime path. +// +// The correctness argument for concurrent consumers is that every ckVtime / +// ckIndex mutation happens inside a single Lua script and Redis serialises +// scripts. These tests check the scripts do not assume any cross-call state: +// two RunQueue instances hammering the same keyspace must still serve every +// message exactly once, never rewind a tag, and leave the vtime state clean. + +const testOptions = { + name: "rq", + tracer: trace.getTracer("rq"), + workers: 1, + defaultEnvConcurrency: 25, + logger: new Logger("RunQueue", "warn"), + retryOptions: { + maxAttempts: 5, + factor: 1.1, + minTimeoutInMs: 100, + maxTimeoutInMs: 1_000, + randomize: true, + }, + keys: new RunQueueFullKeyProducer(), +}; + +const authenticatedEnvDev = { + id: "e1234", + type: "DEVELOPMENT" as const, + maximumConcurrencyLimit: 10, + concurrencyLimitBurstFactor: new Decimal(2.0), + project: { id: "p1234" }, + organization: { id: "o1234" }, +}; + +function createQueue(redisContainer: any, keyPrefix: string, vtimeEnabled: boolean) { + return new RunQueue({ + ...testOptions, + // These tests drive every dequeue themselves (testDequeueFromMasterQueue + + // skipDequeueProcessing). The ONLY concurrency is the explicit consumer + // loops below, so the autonomous master-queue consumers and the background + // worker must stay off in every instance. + masterQueueConsumersDisabled: true, + workerOptions: { disabled: true }, + ckVirtualTimeScheduling: { + enabled: vtimeEnabled, + }, + queueSelectionStrategy: new FairQueueSelectionStrategy({ + redis: { + keyPrefix, + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + keys: testOptions.keys, + }), + redis: { + keyPrefix, + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + }); +} + +function makeMessage(overrides: Partial = {}): InputPayload { + return { + runId: "r1", + taskIdentifier: "task/my-task", + orgId: "o1234", + projectId: "p1234", + environmentId: "e1234", + environmentType: "DEVELOPMENT", + queue: "task/my-task", + timestamp: Date.now(), + attempt: 0, + ...overrides, + }; +} + +// The ckVtime/ckIndex member for a variant is the fully-qualified variant queue +// key (org:proj:env:queue:...:ck:), which is exactly what queueKey() produces. +function variantName(ck: string): string { + return testOptions.keys.queueKey(authenticatedEnvDev, "task/my-task", ck); +} + +vi.setConfig({ testTimeout: 120_000 }); + +describe("CK virtual-time concurrency and op-count budget", () => { + redisTest("two consumers, one base queue, no corruption", async ({ redisContainer }) => { + const keyPrefix = "rq15:"; + // one instance for enqueues, two more (same Redis, same key prefix) as consumers + const producer = createQueue(redisContainer, keyPrefix, true); + const consumerA = createQueue(redisContainer, keyPrefix, true); + const consumerB = createQueue(redisContainer, keyPrefix, true); + try { + const t0 = Date.now() - 100_000; + const cks = ["a", "b", "c", "d", "e", "f"]; + const perKey = 30; + + const enqueuedIds = new Set(); + for (const ck of cks) { + for (let i = 0; i < perKey; i++) { + const runId = `r-${ck}-${i}`; + enqueuedIds.add(runId); + await producer.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId, concurrencyKey: ck, timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(variantName("a")); + const ckVtimeFloorKey = testOptions.keys.ckVtimeFloorKeyFromQueue(variantName("a")); + + // shared across both consumer loops: messageId -> times served + const serveCounts = new Map(); + const floorSamples: number[] = []; + let floorRewind: { consumer: string; prev: number; next: number } | undefined; + + const runConsumer = async (name: string, queue: RunQueue) => { + let prevFloor = 0; + let iterations = 0; + while (serveCounts.size < enqueuedIds.size) { + iterations++; + if (iterations > 600) { + throw new Error( + `consumer ${name}: iteration cap hit with ${serveCounts.size}/${enqueuedIds.size} unique messages served` + ); + } + + const messages = await queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 5); + + // record serves immediately, so the exactly-once bookkeeping covers + // messages currently held by the other consumer too + for (const m of messages) { + serveCounts.set(m.messageId, (serveCounts.get(m.messageId) ?? 0) + 1); + } + + if (messages.length === 0) { + // nothing servable right now (the other consumer holds the slots); + // yield so its hold can elapse + await sleep(2); + } else { + // short hold before acking, so the two loops genuinely overlap + await sleep(3); + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + // sample the floor between iterations: it must never decrease + const floor = Number((await queue.redis.get(ckVtimeFloorKey)) ?? "0"); + if (floor < prevFloor && !floorRewind) { + floorRewind = { consumer: name, prev: prevFloor, next: floor }; + } + prevFloor = floor; + floorSamples.push(floor); + } + }; + + await Promise.all([runConsumer("A", consumerA), runConsumer("B", consumerB)]); + + // exactly once: the union of served IDs equals the enqueued set, no duplicates + const duplicates = [...serveCounts.entries()].filter(([, count]) => count > 1); + expect(duplicates).toEqual([]); + expect(serveCounts.size).toBe(enqueuedIds.size); + expect(new Set(serveCounts.keys())).toEqual(enqueuedIds); + + // the floor never rewound in either consumer's sample sequence + expect(floorRewind).toBeUndefined(); + + // after drain: every variant was GC'd from ckVtime.. + expect(await consumerA.redis.zcard(ckVtimeKey)).toBe(0); + // ..and the floor sits at the max it ever reached + const finalFloor = Number((await consumerA.redis.get(ckVtimeFloorKey)) ?? "0"); + expect(finalFloor).toBe(Math.max(finalFloor, ...floorSamples)); + } finally { + await producer.quit(); + await consumerA.quit(); + await consumerB.quit(); + } + }); + + redisTest("concurrent enqueue during dequeue cannot rewind a tag", async ({ redisContainer }) => { + const queue = createQueue(redisContainer, "rq16:", true); + try { + const t0 = Date.now() - 100_000; + + // hot backlog large enough that it never drains (so it is never GC'd and + // re-registered, keeping the ZSCORE comparison meaningful), plus a + // competitor key so hot is not the only candidate + for (let i = 0; i < 12; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ runId: `r-hot-${i}`, concurrencyKey: "hot", timestamp: t0 + i }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + for (let i = 0; i < 30; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-cold-${i}`, + concurrencyKey: "cold", + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + + const hotVariant = variantName("hot"); + const ckVtimeKey = testOptions.keys.ckVtimeKeyFromQueue(hotVariant); + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + // enqueue registered hot at the initial floor + let prevTag = Number(await queue.redis.zscore(ckVtimeKey, hotVariant)); + expect(prevTag).toBe(0); + + let extra = 0; + for (let round = 0; round < 12; round++) { + // enqueues on the hot key racing a dequeue batch: the enqueue script's + // ZADD NX registration must never rewind the tag the dequeue script is + // advancing (advance-only writes) + const [, messages] = await Promise.all([ + (async () => { + for (let j = 0; j < 2; j++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-hot-extra-${extra++}`, + concurrencyKey: "hot", + timestamp: t0 + 1000 + round, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + })(), + queue.testDequeueFromMasterQueue(shard, authenticatedEnvDev.id, 3), + ]); + + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + + const tag = await queue.redis.zscore(ckVtimeKey, hotVariant); + // never drained, so never GC'd + expect(tag, `round ${round}: hot variant missing from ckVtime`).not.toBeNull(); + expect(Number(tag), `round ${round}: tag rewound`).toBeGreaterThanOrEqual(prevTag); + prevTag = Number(tag); + } + + // hot was actually served along the way (the invariant wasn't vacuous) + expect(prevTag).toBeGreaterThan(0); + } finally { + await queue.quit(); + } + }); + + redisTest("op-count budget: vtime dequeue overhead is bounded", async ({ redisContainer }) => { + const maxCount = 5; + const dequeueCalls = 50; + const cks = ["a", "b", "c", "d", "e", "f"]; + const perKey = 30; + + // second plain ioredis client (no key prefix) for CONFIG RESETSTAT / INFO. + // Redis command stats are server-wide, so each phase resets them after its + // enqueues and reads them right after its 50th dequeue call. + const statsClient = createRedisClient({ + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }); + + // Runs one phase: identical data under a fresh keyspace, then 50 identical + // dequeue calls (ack immediately, so env concurrency never gates a serve + // and both phases fully drain the same 180 messages inside the window). + const runPhase = async (keyPrefix: string, vtimeEnabled: boolean) => { + const queue = createQueue(redisContainer, keyPrefix, vtimeEnabled); + try { + const t0 = Date.now() - 100_000; + for (const ck of cks) { + for (let i = 0; i < perKey; i++) { + await queue.enqueueMessage({ + env: authenticatedEnvDev, + message: makeMessage({ + runId: `r-${ck}-${i}`, + concurrencyKey: ck, + timestamp: t0 + i, + }), + workerQueue: authenticatedEnvDev.id, + skipDequeueProcessing: true, + }); + } + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(authenticatedEnvDev.id, 2); + + await statsClient.call("CONFIG", "RESETSTAT"); + + let served = 0; + for (let call = 0; call < dequeueCalls; call++) { + const messages = await queue.testDequeueFromMasterQueue( + shard, + authenticatedEnvDev.id, + maxCount + ); + served += messages.length; + for (const m of messages) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, m.messageId, { + skipDequeueProcessing: true, + }); + } + } + + const info = await statsClient.info("commandstats"); + return { served, totalCalls: totalCommandCalls(info) }; + } finally { + await queue.quit(); + } + }; + + try { + const off = await runPhase("rq17off:", false); + const on = await runPhase("rq17on:", true); + + // both phases did identical work: the full 180 messages served and acked + expect(off.served).toBe(cks.length * perKey); + expect(on.served).toBe(cks.length * perKey); + + // Per dequeue call the vtime path adds at worst 7 fixed ops: GET floor, + // ZRANGE min, ZRANGE window, the pass-2 ZRANGEBYSCORE, SET floor, + // EXISTS ckVtime, EXPIRE ckVtime — plus per serve one ZSCORE and one ZADD. + const budget = dequeueCalls * (7 + 2 * maxCount); + expect( + on.totalCalls, + `on_total ${on.totalCalls} exceeds off_total ${off.totalCalls} + budget ${budget}` + ).toBeLessThanOrEqual(off.totalCalls + budget); + } finally { + await statsClient.quit(); + } + }); +}); + +// Sums calls= across every cmdstat_ line of INFO commandstats. Includes +// commands executed from inside Lua scripts, which is exactly what we want: +// the vtime overhead lives in the dequeue script body. +function totalCommandCalls(info: string): number { + let total = 0; + for (const line of info.split("\n")) { + const match = line.match(/^cmdstat_[^:]+:calls=(\d+)/); + if (match) { + total += Number(match[1]); + } + } + return total; +} diff --git a/internal-packages/run-engine/src/run-queue/tests/ckVtimeFairness.test.ts b/internal-packages/run-engine/src/run-queue/tests/ckVtimeFairness.test.ts new file mode 100644 index 00000000000..622b2cdd247 --- /dev/null +++ b/internal-packages/run-engine/src/run-queue/tests/ckVtimeFairness.test.ts @@ -0,0 +1,531 @@ +import { redisTest } from "@internal/testcontainers"; +import { trace } from "@internal/tracing"; +import { Logger } from "@trigger.dev/core/logger"; +import { Decimal } from "@trigger.dev/database"; +import { appendFileSync } from "node:fs"; +import { describe } from "node:test"; +import { FairQueueSelectionStrategy } from "../fairQueueSelectionStrategy.js"; +import { RunQueue } from "../index.js"; +import { RunQueueFullKeyProducer } from "../keyProducer.js"; +import type { InputPayload } from "../types.js"; + +// Fairness scenarios driven through the REAL batched dequeue path (maxCount 10), +// closing the spike's maxCount=1 fidelity gap. Every assertion is a ratio between +// a flag-ON and a flag-OFF run of the same scenario (identical enqueue order and +// timestamps), so the tests are stable in CI. +// +// Scenario shapes are ported from the throwaway fairness spike (ckScenarios.ts / +// capsFairness.bench.test.ts): the message counts and head-age structure are +// copied as values, nothing is imported from the spike. +// +// Harness: a deterministic step loop. All messages are enqueued before step 0 +// with explicit past timestamps (a pre-existing backlog), so every message's +// logical arrival step is 0 and its wait is simply the step it was served at. +// Each step makes one dequeue call with maxCount 10, records the serves, then +// acks in-flight messages whose logical hold has elapsed (servedAt + hold <= +// step), which is how the env concurrency contends across steps. No wall-clock +// sleeps and no randomness anywhere. + +const testOptions = { + name: "rq", + tracer: trace.getTracer("rq"), + workers: 1, + defaultEnvConcurrency: 25, + logger: new Logger("RunQueue", "warn"), + retryOptions: { + maxAttempts: 5, + factor: 1.1, + minTimeoutInMs: 100, + maxTimeoutInMs: 1_000, + randomize: true, + }, + keys: new RunQueueFullKeyProducer(), +}; + +const authenticatedEnvDev = { + id: "e1234", + type: "DEVELOPMENT" as const, + maximumConcurrencyLimit: 10, + concurrencyLimitBurstFactor: new Decimal(2.0), + project: { id: "p1234" }, + organization: { id: "o1234" }, +}; + +function createQueue(redisContainer: any, keyPrefix: string, vtimeEnabled: boolean) { + return new RunQueue({ + ...testOptions, + // The step loop drives every op itself (testDequeueFromMasterQueue + + // skipDequeueProcessing), so the autonomous master-queue consumers and the + // background worker must not race it. + masterQueueConsumersDisabled: true, + workerOptions: { disabled: true }, + ckVirtualTimeScheduling: { + enabled: vtimeEnabled, + }, + queueSelectionStrategy: new FairQueueSelectionStrategy({ + redis: { + keyPrefix, + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + keys: testOptions.keys, + }), + redis: { + keyPrefix, + host: redisContainer.getHost(), + port: redisContainer.getPort(), + }, + }); +} + +function makeMessage(overrides: Partial = {}): InputPayload { + return { + runId: "r1", + taskIdentifier: "task/my-task", + orgId: "o1234", + projectId: "p1234", + environmentId: "e1234", + environmentType: "DEVELOPMENT", + queue: "task/my-task", + timestamp: Date.now(), + attempt: 0, + ...overrides, + }; +} + +type ScenarioMessage = { runId: string; ck: string; timestamp: number }; + +type Scenario = { + name: string; + messages: ScenarioMessage[]; + // Effective env concurrency for the run (burst factor is pinned to 1.0). + // This is the contention knob: it caps how many serves fit in one dequeue + // call (actualMaxCount = min(maxCount, available env capacity)). + envConcurrencyLimit: number; + // Logical hold: a served message occupies its env slot until the end of + // step servedAt + holdSteps, when it is acked. + holdSteps: number; + // Safety cap so a work-conservation bug fails the count assertions instead + // of hanging the test. + maxSteps: number; +}; + +type ServeRecord = { step: number; ck: string; messageId: string }; + +type ScenarioResult = { + serves: ServeRecord[]; + // step at which the last message was served + drainStep: number; + // serves that happened in steps where >= 2 keys still had queued backlog + contentionServes: { total: number; byCk: Map }; +}; + +async function runScenario( + redisContainer: any, + scenario: Scenario, + vtimeEnabled: boolean +): Promise { + // Separate key prefix per run: the ON and OFF runs of a scenario share one + // Redis container but never share state. + const keyPrefix = `runqueue:test:${scenario.name}:${vtimeEnabled ? "on" : "off"}:`; + const queue = createQueue(redisContainer, keyPrefix, vtimeEnabled); + + try { + const env = { + ...authenticatedEnvDev, + maximumConcurrencyLimit: scenario.envConcurrencyLimit, + concurrencyLimitBurstFactor: new Decimal(1), + }; + await queue.updateEnvConcurrencyLimits(env); + + for (const msg of scenario.messages) { + await queue.enqueueMessage({ + env, + message: makeMessage({ + runId: msg.runId, + concurrencyKey: msg.ck, + timestamp: msg.timestamp, + }), + workerQueue: env.id, + skipDequeueProcessing: true, + }); + } + + const shard = testOptions.keys.masterQueueShardForEnvironment(env.id, 2); + const total = scenario.messages.length; + + const remaining = new Map(); + for (const m of scenario.messages) { + remaining.set(m.ck, (remaining.get(m.ck) ?? 0) + 1); + } + + const serves: ServeRecord[] = []; + const inFlight: { messageId: string; servedAtStep: number }[] = []; + const contentionServes = { total: 0, byCk: new Map() }; + let drainStep = -1; + + for (let step = 0; step < scenario.maxSteps && serves.length < total; step++) { + // evaluated before the dequeue: does this step have cross-key contention? + let keysWithBacklog = 0; + for (const count of remaining.values()) { + if (count > 0) keysWithBacklog++; + } + + const messages = await queue.testDequeueFromMasterQueue(shard, env.id, 10); + + for (const m of messages) { + const ck = m.message.concurrencyKey ?? ""; + serves.push({ step, ck, messageId: m.messageId }); + remaining.set(ck, (remaining.get(ck) ?? 0) - 1); + inFlight.push({ messageId: m.messageId, servedAtStep: step }); + if (keysWithBacklog >= 2) { + contentionServes.total++; + contentionServes.byCk.set(ck, (contentionServes.byCk.get(ck) ?? 0) + 1); + } + if (serves.length === total) { + drainStep = step; + } + } + + // release env/queue concurrency for serves whose hold has elapsed + for (let i = inFlight.length - 1; i >= 0; i--) { + const entry = inFlight[i]!; + if (entry.servedAtStep + scenario.holdSteps <= step) { + await queue.acknowledgeMessage(authenticatedEnvDev.organization.id, entry.messageId, { + skipDequeueProcessing: true, + }); + inFlight.splice(i, 1); + } + } + } + + return { serves, drainStep, contentionServes }; + } finally { + await queue.quit(); + } +} + +// Wait per message = serve step - arrival step, and arrival is step 0 for the +// whole pre-enqueued backlog, so the wait is just the serve step. +function meanWait(result: ScenarioResult, matches: (ck: string) => boolean): number { + const waits = result.serves.filter((s) => matches(s.ck)).map((s) => s.step); + expect(waits.length).toBeGreaterThan(0); + return waits.reduce((a, b) => a + b, 0) / waits.length; +} + +function firstServeStep(result: ScenarioResult, matches: (ck: string) => boolean): number { + const first = result.serves.find((s) => matches(s.ck)); + expect(first).toBeDefined(); + return first!.step; +} + +// No loss and no double-serve, in both runs. +function assertConservation(scenario: Scenario, on: ScenarioResult, off: ScenarioResult) { + expect(on.serves.length).toBe(scenario.messages.length); + expect(off.serves.length).toBe(scenario.messages.length); + expect(new Set(on.serves.map((s) => s.messageId)).size).toBe(scenario.messages.length); + expect(new Set(off.serves.map((s) => s.messageId)).size).toBe(scenario.messages.length); +} + +function debugLog(name: string, data: Record) { + if (process.env.CK_FAIRNESS_DEBUG) { + // the test reporter swallows console output, so append to a file instead + appendFileSync( + process.env.CK_FAIRNESS_DEBUG, + `[ckVtimeFairness] ${name} ${JSON.stringify(data)}\n` + ); + } +} + +vi.setConfig({ testTimeout: 120_000 }); + +describe("CK virtual-time fairness on the real batched dequeue path", () => { + // ckSkew (spike shape): one heavy key with a 120-message backlog on an old + // shared head, 4 light keys with 10 messages each on later heads. + // + // Contention regime: env limit 1, hold 3. The batched dequeue serves at most + // one message per variant per call, so at env limit 4 (the spike's driver + // setting) a single heavy key cannot crowd out 4 light keys at all: both + // flags serve every light key each round and the ON/OFF ratio sits near 1. + // The head-age starvation the spike measured appears on this path when env + // capacity serializes the calls (limit 1): flag OFF then always picks the + // globally oldest head, which is heavy for its whole backlog. + redisTest( + "ckSkew: light keys stop waiting behind the heavy backlog", + async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const messages: ScenarioMessage[] = []; + for (let i = 0; i < 120; i++) { + messages.push({ runId: `heavy-${i}`, ck: "heavy", timestamp: t0 }); + } + for (let i = 0; i < 10; i++) { + for (let k = 0; k < 4; k++) { + messages.push({ + runId: `light${k}-${i}`, + ck: `light${k}`, + timestamp: t0 + 10_000 + i * 4 + k, + }); + } + } + const scenario: Scenario = { + name: "ckSkew", + messages, + envConcurrencyLimit: 1, + holdSteps: 3, + maxSteps: 1_000, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + assertConservation(scenario, on, off); + + const isLight = (ck: string) => ck.startsWith("light"); + const onWait = meanWait(on, isLight); + const offWait = meanWait(off, isLight); + debugLog("ckSkew", { onWait, offWait, ratio: onWait / offWait }); + + // Heavy's wait may rise under the fair order; that is expected and not + // asserted down. + expect(onWait).toBeLessThanOrEqual(0.3 * offWait); + } + ); + + // ckTrickle (spike shape): one bulk key with a 120-message backlog on an old + // shared head, two trickle keys with 15 messages each on later heads. Same + // serialized contention regime as ckSkew, same assertion. + redisTest( + "ckTrickle: trickle keys stop waiting behind the bulk backlog", + async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const messages: ScenarioMessage[] = []; + for (let i = 0; i < 120; i++) { + messages.push({ runId: `bulk-${i}`, ck: "bulk", timestamp: t0 }); + } + for (let i = 0; i < 15; i++) { + for (let k = 0; k < 2; k++) { + messages.push({ + runId: `trickle${k}-${i}`, + ck: `trickle${k}`, + timestamp: t0 + 10_000 + i * 2 + k, + }); + } + } + const scenario: Scenario = { + name: "ckTrickle", + messages, + envConcurrencyLimit: 1, + holdSteps: 3, + maxSteps: 1_000, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + assertConservation(scenario, on, off); + + const isTrickle = (ck: string) => ck.startsWith("trickle"); + const onWait = meanWait(on, isTrickle); + const offWait = meanWait(off, isTrickle); + debugLog("ckTrickle", { onWait, offWait, ratio: onWait / offWait }); + + expect(onWait).toBeLessThanOrEqual(0.3 * offWait); + } + ); + + // ckSybil (spike shape, the case per-key caps cannot fix): 20 attacker keys + // with 8 messages each, all on older heads, and 1 light key with 10 newer + // messages. 21 variants against a batch of 10 exercises the batched path + // properly: flag OFF walks the age order and only reaches the light key when + // the attackers are nearly drained; flag ON serves the light key from the + // floor on its first fair round. + redisTest("ckSybil: many attacker keys cannot starve a light key", async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const messages: ScenarioMessage[] = []; + for (let i = 0; i < 8; i++) { + for (let k = 0; k < 20; k++) { + const ck = `att${String(k).padStart(2, "0")}`; + messages.push({ runId: `${ck}-${i}`, ck, timestamp: t0 + i * 20 + k }); + } + } + for (let i = 0; i < 10; i++) { + messages.push({ runId: `light-${i}`, ck: "light", timestamp: t0 + 50_000 + i }); + } + const scenario: Scenario = { + name: "ckSybil", + messages, + envConcurrencyLimit: 25, + holdSteps: 3, + maxSteps: 300, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + assertConservation(scenario, on, off); + + const isLight = (ck: string) => ck === "light"; + + // Reachability at the floor: enqueue registered the light key at the + // floor, so it is served within the first 3 steps even though 20 attacker + // variants sit ahead of it in age order. + const onFirstServe = firstServeStep(on, isLight); + expect(onFirstServe).toBeLessThanOrEqual(2); + + const onWait = meanWait(on, isLight); + const offWait = meanWait(off, isLight); + + // Contention-window share (directional, per the spike's confounding + // caveat; the wait ratio is the headline): over the steps where >= 2 keys + // had queued backlog, light's served fraction is at least half its fair + // share of 1/21. + const lightContentionServes = on.contentionServes.byCk.get("light") ?? 0; + const lightShare = lightContentionServes / on.contentionServes.total; + + debugLog("ckSybil", { + onWait, + offWait, + ratio: onWait / offWait, + onFirstServe, + lightShare, + fairShare: 1 / 21, + }); + + expect(onWait).toBeLessThanOrEqual(0.7 * offWait); + expect(lightShare).toBeGreaterThanOrEqual(0.5 * (1 / 21)); + }); + + // ckBalanced (spike shape, no-harm check): 4 symmetric keys with 25 messages + // each. The fair order must not make the symmetric case worse. + redisTest( + "ckBalanced: fair order does not hurt the symmetric case", + async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const cks = ["bal0", "bal1", "bal2", "bal3"]; + const messages: ScenarioMessage[] = []; + for (let i = 0; i < 25; i++) { + for (let k = 0; k < cks.length; k++) { + messages.push({ + runId: `${cks[k]}-${i}`, + ck: cks[k]!, + timestamp: t0 + i * 4 + k, + }); + } + } + const scenario: Scenario = { + name: "ckBalanced", + messages, + envConcurrencyLimit: 4, + holdSteps: 3, + maxSteps: 500, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + assertConservation(scenario, on, off); + + const maxPerKeyMeanWait = (result: ScenarioResult) => + Math.max(...cks.map((ck) => meanWait(result, (c) => c === ck))); + + const onMax = maxPerKeyMeanWait(on); + const offMax = maxPerKeyMeanWait(off); + debugLog("ckBalanced", { onMax, offMax, ratio: onMax / offMax }); + + // Observed ratio is 1.0 (the fair order is neutral on the symmetric case), + // so allow only modest headroom rather than the original 1.25. + expect(onMax).toBeLessThanOrEqual(1.1 * offMax); + } + ); + + // ckManyKeys (sharding coverage): cardinality ABOVE the pass-1 fair window. + // The batched dequeue uses maxCount 10, so window = actualMaxCount * 3 = 30. + // With ~60 attacker keys (all on the same old head) plus 1 light key on a + // newer head, 61 variants sit above the 30-wide pass-1 ZRANGE window, so no + // single fair pass can even see every key. The property to hold is that this + // does NOT permanently starve the light key: as attackers advance their tags + // out of the bottom of the window, the light key (still at the floor) rises + // into it and gets served, and every message drains exactly once. A bounded + // first-serve delay is fine; permanent starvation or a stuck drain is not. + redisTest( + "ckManyKeys: light key is not starved when cardinality exceeds the fair window", + async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const messages: ScenarioMessage[] = []; + const attackerCount = 60; + for (let i = 0; i < 8; i++) { + for (let k = 0; k < attackerCount; k++) { + const ck = `att${String(k).padStart(2, "0")}`; + // All attackers share the same old head timestamp (tied heads). + messages.push({ runId: `${ck}-${i}`, ck, timestamp: t0 }); + } + } + for (let i = 0; i < 10; i++) { + messages.push({ runId: `light-${i}`, ck: "light", timestamp: t0 + 50_000 + i }); + } + const scenario: Scenario = { + name: "ckManyKeys", + messages, + envConcurrencyLimit: 25, + holdSteps: 3, + maxSteps: 1_000, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + // No loss and no double-serve in either run: the run terminates and every + // message (attackers + light) is served exactly once within maxSteps. + assertConservation(scenario, on, off); + + const isLight = (ck: string) => ck === "light"; + + // The light key IS eventually served (no permanent starvation) in both + // runs, and drains fully. + const onFirstServe = firstServeStep(on, isLight); + const offFirstServe = firstServeStep(off, isLight); + expect(on.drainStep).toBeGreaterThanOrEqual(0); + expect(off.drainStep).toBeGreaterThanOrEqual(0); + + debugLog("ckManyKeys", { + variants: attackerCount + 1, + onFirstServe, + offFirstServe, + onDrainStep: on.drainStep, + offDrainStep: off.drainStep, + }); + } + ); + + // ckHeavyIdle (spike shape, work conservation): a single key with 60 + // messages and nothing else contending. Any extra step to drain under the + // fair order is a work-conservation bug, so the step counts must be exactly + // equal. + redisTest( + "ckHeavyIdle: a lone key drains in exactly the same steps", + async ({ redisContainer }) => { + const t0 = Date.now() - 500_000; + const messages: ScenarioMessage[] = []; + for (let i = 0; i < 60; i++) { + messages.push({ runId: `solo-${i}`, ck: "solo", timestamp: t0 + i }); + } + const scenario: Scenario = { + name: "ckHeavyIdle", + messages, + envConcurrencyLimit: 25, + holdSteps: 3, + maxSteps: 300, + }; + + const on = await runScenario(redisContainer, scenario, true); + const off = await runScenario(redisContainer, scenario, false); + + assertConservation(scenario, on, off); + + debugLog("ckHeavyIdle", { onDrainStep: on.drainStep, offDrainStep: off.drainStep }); + + expect(on.drainStep).toBeGreaterThanOrEqual(0); + expect(on.drainStep).toBe(off.drainStep); + } + ); +}); diff --git a/internal-packages/run-engine/src/run-queue/tests/keyProducer.test.ts b/internal-packages/run-engine/src/run-queue/tests/keyProducer.test.ts index 3e31085d678..1f61ed1cfc7 100644 --- a/internal-packages/run-engine/src/run-queue/tests/keyProducer.test.ts +++ b/internal-packages/run-engine/src/run-queue/tests/keyProducer.test.ts @@ -432,4 +432,19 @@ describe("KeyProducer", () => { "{org:o1234}:proj:p1234:env:e1234:queue:task/foo:ck:*" ); }); + + it("produces ckVtime keys from a CK variant queue name", () => { + const keyProducer = new RunQueueFullKeyProducer(); + const q = "{org:o1}:proj:p1:env:e1:queue:task/my-task:ck:tenant-a"; + expect(keyProducer.ckVtimeKeyFromQueue(q)).toBe( + "{org:o1}:proj:p1:env:e1:queue:task/my-task:ckVtime" + ); + expect(keyProducer.ckVtimeFloorKeyFromQueue(q)).toBe( + "{org:o1}:proj:p1:env:e1:queue:task/my-task:ckVtimeFloor" + ); + // ck wildcard and base-queue inputs normalise the same way + expect(keyProducer.ckVtimeKeyFromQueue(q.replace(":ck:tenant-a", ":ck:*"))).toBe( + keyProducer.ckVtimeKeyFromQueue(q) + ); + }); }); diff --git a/internal-packages/run-engine/src/run-queue/types.ts b/internal-packages/run-engine/src/run-queue/types.ts index 0905f3971de..a98051e76a7 100644 --- a/internal-packages/run-engine/src/run-queue/types.ts +++ b/internal-packages/run-engine/src/run-queue/types.ts @@ -129,6 +129,8 @@ export interface RunQueueKeyProducer { // CK index methods ckIndexKeyFromQueue(queue: string): string; + ckVtimeKeyFromQueue(queue: string): string; + ckVtimeFloorKeyFromQueue(queue: string): string; baseQueueKeyFromQueue(queue: string): string; isCkWildcard(queue: string): boolean; toCkWildcard(queue: string): string;