diff --git a/docs/superpowers/plans/2026-07-23-ck-virtual-time-scheduling-plan.md b/docs/superpowers/plans/2026-07-23-ck-virtual-time-scheduling-plan.md new file mode 100644 index 0000000000..2865c0ba1c --- /dev/null +++ b/docs/superpowers/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: +`docs/superpowers/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-XX-XX-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 docs/superpowers/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-XX-XX-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/docs/superpowers/references/README.md b/docs/superpowers/references/README.md new file mode 100644 index 0000000000..e16ab19a9d --- /dev/null +++ b/docs/superpowers/references/README.md @@ -0,0 +1,43 @@ +# Run-queue multi-tenant fairness: spike references + +Reference material for the implementation plan +`docs/superpowers/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/docs/superpowers/references/run-queue-fairness-base-queue-findings.md b/docs/superpowers/references/run-queue-fairness-base-queue-findings.md new file mode 100644 index 0000000000..e386aba8d5 --- /dev/null +++ b/docs/superpowers/references/run-queue-fairness-base-queue-findings.md @@ -0,0 +1,168 @@ +# Fair-queueing spike: findings + +This is a throwaway spike. It ships nothing and should be deleted before any +merge to main; it exists to inform a design decision, not to become 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/docs/superpowers/references/run-queue-fairness-caps-vs-scheduling-findings.md b/docs/superpowers/references/run-queue-fairness-caps-vs-scheduling-findings.md new file mode 100644 index 0000000000..b1daf94ee8 --- /dev/null +++ b/docs/superpowers/references/run-queue-fairness-caps-vs-scheduling-findings.md @@ -0,0 +1,198 @@ +# Caps vs scheduling: reconciliation findings + +Throwaway spike. Ships nothing; delete before any merge to main. 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/docs/superpowers/references/run-queue-fairness-ck-findings.md b/docs/superpowers/references/run-queue-fairness-ck-findings.md new file mode 100644 index 0000000000..a0e806fd62 --- /dev/null +++ b/docs/superpowers/references/run-queue-fairness-ck-findings.md @@ -0,0 +1,122 @@ +# Per-concurrency-key fairness spike: findings + +Throwaway spike. Ships nothing; delete before any merge to main. + +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/docs/superpowers/references/run-queue-fairness-research.md b/docs/superpowers/references/run-queue-fairness-research.md new file mode 100644 index 0000000000..4539c2a412 --- /dev/null +++ b/docs/superpowers/references/run-queue-fairness-research.md @@ -0,0 +1,133 @@ +# Queue-fairness research (grounding for the caps-vs-scheduling reconciliation) + +Throwaway spike notes. 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.