Skip to content

Commit b9a6557

Browse files
committed
test(run-engine): address CK review (real age scenarios, honesty)
Blind two-model review found the ckSkew/ckBalanced baseline starvation was a Redis lexicographic tie-break on equal enqueue timestamps, not age order. Redesigned scenarios so the backlog key keeps a genuinely old head and other keys arrive via poisson, so the baseline now exercises real age-order starvation (ckSkew baseline 0.187, worst key waits 1321ms; SFQ cuts it to 16ms). FINDINGS corrected: lead with per-key wait (contention share is volume-confounded for low-volume keys), and add the key fidelity limit the review surfaced: the harness serves one key per Lua call, while production batches and re-scores a served key mid-call, so this proves the ordering fix only at maxCount=1 and a production fix must advance per-key state inside the batched Lua. Softened 'viable' to 'worth a design spike'.
1 parent b8d21d2 commit b9a6557

4 files changed

Lines changed: 461 additions & 425 deletions

File tree

internal-packages/run-engine/src/run-queue/fairness-spike-ck/FINDINGS.md

Lines changed: 105 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -2,98 +2,121 @@
22

33
Throwaway spike. Ships nothing; delete before any merge to main.
44

5-
Bottom line: the base-queue spike's ranking carries over to the real seam, and
6-
more starkly. At the concurrency-key grain the production baseline (serve the
7-
oldest-head CK first) starves other keys outright, and virtual-time (SFQ) and
8-
stride fix it. DRR is close. A CoDel wrapper on top of the broken baseline makes
9-
it worse. Crucially, this was measured by driving the real
10-
`dequeueMessagesFromCkQueueTracked` Lua and its real concurrency gating; only the
11-
`ckIndex` scores were rewritten to express each discipline, so the result says a
12-
production fix is viable (advance the ckIndex score by a fair discipline instead
13-
of by head timestamp).
14-
15-
## What was driven, and the one affordance
16-
17-
Runs enqueue across many concurrency keys under a single base queue via the real
18-
`RunQueue`. The per-CK pick is `ZRANGEBYSCORE ckIndexKey -inf now` inside the CK
19-
Lua (lowest score first, score = head timestamp). For candidates the driver
20-
rewrites those scores each round to encode the discipline's order; the baseline
21-
leaves them as the Lua maintains them, which is production behaviour. Enqueue,
22-
dequeue, per-CK and env concurrency, and ack all run through the real code. The
23-
rescore sweep and an `onServiced(key)` hook are the only affordances; in
24-
production that advance would live in the Lua that maintains `ckIndex`. A smoke
25-
test confirms the baseline path reproduces age-order starvation.
26-
27-
Fairness is measured as in the base-queue spike (arrival-aware contention share
28-
and per-key wait), carrying forward its corrected metric and monotonic-floor
29-
disciplines. All keys are equal weight (concurrency keys carry no configured
30-
weight in production).
5+
Bottom line: the base-queue spike's direction carries over to the real seam. At
6+
the concurrency-key grain the production baseline (serve the oldest-head CK
7+
first) starves keys that arrive behind a big backlog, and virtual-time (SFQ) and
8+
stride fix it: they cut the starved key's wait from ~1300ms to ~20ms by making
9+
the backlog key wait its turn. DRR does the same. A CoDel wrapper on the baseline
10+
makes it worse. This was measured by driving the real
11+
`dequeueMessagesFromCkQueueTracked` Lua and only rewriting `ckIndex` scores to
12+
express each discipline. That is enough to say the ordering fix is worth a design
13+
spike, but NOT that a production implementation is proven (see the fidelity
14+
caveat: the spike serves one key per Lua call, and production dequeues in
15+
batches).
16+
17+
## What was driven, and the two things to know before reading numbers
18+
19+
Runs enqueue across many concurrency keys under one base queue via the real
20+
`RunQueue`. The per-CK pick is `ZRANGEBYSCORE ckIndexKey -inf now` in the CK Lua
21+
(lowest score first, score = head timestamp). Candidates rewrite those scores
22+
each round to encode discipline order; the baseline leaves them (production age
23+
order). Enqueue, dequeue, concurrency gating and ack all run through the real
24+
code.
25+
26+
Two caveats a review forced, both load-bearing:
27+
28+
1. Lead with wait, not contention share. The contention-share metric is
29+
volume-confounded for low-volume keys (a key with 15 runs cannot take a third
30+
of a long window even when served instantly), so on these scenarios it lands
31+
around 0.7 to 0.9 for a discipline that has in fact eliminated the starvation.
32+
The per-key wait is the clean signal.
33+
2. The scenarios must give keys genuinely different head ages. An earlier version
34+
enqueued every run at one timestamp; with tied `ckIndex` scores the real Lua
35+
falls back to a lexicographic member-name tie-break, so the "baseline starves
36+
the heavy key's rivals" result was actually "Redis sorts by name" and the
37+
heavy key only won because "heavy" sorts before "light". Fixed: the backlog
38+
key fires at once (persistently old head) and the other keys arrive via
39+
poisson (distinct, later heads), so the baseline now exercises real age order.
3140

3241
## Results
3342

34-
`contWorstS/W` mean over 3 seeds (min..max). Higher is fairer.
43+
`contWorstS/W` mean over 3 seeds (min..max), and the worst-served key's mean wait
44+
(seed-a, logical ms). Read the wait column as the headline.
3545

36-
| scenario | baseline | sfq | drr | stride | codel(sfq) | codel(baseline) |
37-
| ----------- | ------------------- | ------------------- | ------------------- | ------ | ---------- | --------------- |
38-
| ckSkew | 0.000 | 0.982 (0.946..1.000)| 0.974 | 0.982 | 0.982 | 0.000 |
39-
| ckBalanced | 0.000 | 0.987 | 0.987 | 0.987 | 0.987 | 0.000 |
40-
| ckTrickle | 0.279 (0.254..0.291)| 0.909 (0.891..0.918)| 0.790 (0.769..0.818)| 0.909 | 0.909 | 0.018 |
46+
| scenario | discipline | contWorstS/W | worst-key wait | backlog-key wait |
47+
| ---------- | ---------- | ------------------- | -------------- | ---------------- |
48+
| ckSkew | baseline | 0.187 (0.186..0.188)| 1321 | 872 |
49+
| ckSkew | sfq | 0.723 (0.655..0.769)| 16 | 1150 |
50+
| ckSkew | drr | 0.608 (0.556..0.648)| 21 | 1149 |
51+
| ckTrickle | baseline | 0.279 (0.254..0.291)| 1339 | 872 |
52+
| ckTrickle | sfq | 0.909 (0.891..0.918)| 24 | 1237 |
53+
| ckTrickle | drr | 0.790 (0.769..0.818)| 30 | 1236 |
4154

42-
Per-key mean wait (seed-a, logical ms):
55+
Full matrix (contWorstS/W mean over 3 seeds):
4356

44-
| scenario / discipline | starved-key wait | bulk-key wait |
45-
| --------------------- | ---------------- | ------------- |
46-
| ckSkew baseline | 1853 | 872 |
47-
| ckSkew sfq | 265 | 1328 |
48-
| ckTrickle baseline | 1252 | 872 |
49-
| ckTrickle sfq | 22 | 1237 |
50-
51-
Baseline age-order drives a light key's contention share to 0 on ckSkew (it is
52-
served only after the heavy key drains) and its wait to 1853ms; SFQ cuts that to
53-
265ms, and on ckTrickle from 1252 to 22ms, by making the bulk key wait its turn.
57+
| scenario | baseline | sfq | drr | stride | codel(sfq) | codel(baseline) |
58+
| ---------- | ------------------- | ------------------- | ------------------- | ------ | ---------- | ------------------- |
59+
| ckSkew | 0.187 | 0.723 | 0.608 | 0.723 | 0.723 | 0.104 (0.000..0.157)|
60+
| 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 |
61+
| ckTrickle | 0.279 | 0.909 | 0.790 | 0.909 | 0.909 | 0.018 (0.000..0.055)|
5462

5563
## Verdict per discipline (at the concurrency-key grain)
5664

57-
- SFQ / stride: fix the starvation (about 0.98 on skew/balanced, 0.909 on
58-
trickle), seed-stable, and cut the starved key's wait 7x (skew) to 57x
59-
(trickle). Same as the base-queue spike. Recommended.
60-
- DRR: close behind (0.974 skew, 0.987 balanced, 0.790 trickle). The trickle gap
61-
is the same batch-drain interaction noted in the base-queue spike.
62-
- CoDel(sfq): no harm here, matches SFQ. Unlike the base-queue trickle scenario
63-
it did not overshoot, but it also added nothing; the fair base already bounds
64-
sojourn.
65-
- CoDel(baseline): harmful. Hoisting stale keys on top of the broken baseline
66-
drove ckTrickle to 0.018 (worse than baseline's 0.279). A staleness monitor is
67-
not a substitute for a fair base.
68-
- Baseline (production age order): starves keys at this grain. Worse than the
69-
base-queue proxy (0.000 vs 0.288 there), because a concurrency key with a
70-
backlog of old heads is served to exhaustion before newer keys get a turn. This
71-
is #2617 measured at the seam where it actually lives.
72-
73-
## Caveats
74-
75-
- The rescore sweep stands in for a production change to how `ckIndex` scores are
76-
maintained. The spike proves the ordering fix works through the real dequeue
77-
Lua and concurrency gating; it does not implement or measure the production
78-
wiring (which would advance the score inside the enqueue/dequeue Lua and hold
79-
per-key discipline state in Redis, not process memory).
80-
- ckBalanced baseline reading 0.000 is partly a tie-break artifact: with equal
81-
enqueue timestamps the age order is a deterministic member-name tie-break that
82-
parks one key last during a short contention window. The direction (baseline
83-
unfair, candidates fair) is right; lean on ckSkew and the wait numbers for the
84-
magnitude.
65+
- SFQ / stride: fix the starvation. Contention share improves (skew 0.187 to
66+
0.723, trickle 0.279 to 0.909) and the starved key's wait collapses (skew 1321
67+
to 16, trickle 1339 to 24) because the backlog key now waits its turn (its wait
68+
rises 872 to ~1150 to 1237). Identical to each other on every scenario.
69+
Recommended discipline for the fix.
70+
- DRR: fixes the wait just as well (skew 21, trickle 30) and its contention share
71+
tracks SFQ within noise (sometimes a little lower, sometimes higher, e.g.
72+
balanced 0.730 vs 0.611). Fine.
73+
- CoDel(sfq): no harm, matches SFQ to the decimal. Adds nothing on top of a fair
74+
base.
75+
- CoDel(baseline): harmful. Hoisting stale keys on top of the age-order baseline
76+
drove ckSkew to 0.104 (below baseline's 0.187) and ckTrickle to 0.018 (below
77+
0.279). A staleness monitor is not a substitute for a fair base.
78+
- Baseline (production age order): starves keys that queue behind a backlog
79+
(ckSkew 0.187, worst key waits 1321ms; ckTrickle 0.279, 1339ms). It is roughly
80+
fair when keys are symmetric (ckBalanced 0.515, though seed-variant 0.444 to
81+
0.600). This is the #2617 dynamic at the seam where it lives.
82+
83+
## Fidelity caveat (the reason this is not "proven for production")
84+
85+
The driver dequeues one key per Lua call (`maxCount = 1`) and rescores `ckIndex`
86+
before each call. Production dequeues in batches (`maxCount` default 10). Inside a
87+
batched CK-dequeue call the Lua re-scores each served key back to its head
88+
timestamp as it goes, so a once-per-round rescore would only steer the FIRST pick
89+
of a batch; the rest would follow head-timestamp order again. So this spike
90+
demonstrates the ordering fix only in a one-key-per-call regime, which is not how
91+
production dequeues. A real fix has to advance per-key discipline state inside the
92+
Lua on every serve (and hold that state in Redis, not process memory). This spike
93+
does not exercise that batch path, so the correct claim is "the ordering fix is
94+
worth a design spike", not "a production fix is viable".
95+
96+
## Other caveats
97+
98+
- Contention share is volume-confounded (see above); the wait column is the
99+
trustworthy signal, and the contention numbers should be read as directional.
100+
- The DRR contention-share gap is NOT the base-queue spike's batch-drain artifact
101+
(that harness batched; this one serves one key per call and advances DRR's
102+
deficit every serve). The cause of DRR's slightly lower share here is not
103+
established; its wait result is as good as SFQ's.
104+
- Per-CK concurrency gating never binds in these runs (no per-CK limit is set, so
105+
it collapses to the env limit), so the spike says nothing about the per-CK
106+
concurrency-limit-multiplication half of #2617, which is out of scope.
107+
- A rescore discipline advances its floor/ring state on the final no-op drain
108+
round of an instant (order() is called before the empty dequeue). It is
109+
self-correcting and does not corrupt the event-based metrics, but it is a minor
110+
infidelity to a production per-serve advance.
85111
- Equal weights only; single shard, single base queue, single sequential
86-
consumer; simulated holds on a logical clock; 3 seeds (shows the baseline's
87-
failure and the virtual-time schemes' stability, not a statistical study).
88-
- The other half of #2617, per-CK concurrency-limit multiplication, is a limit
89-
problem not a dequeue-ordering one, and is out of scope here.
112+
consumer; simulated holds on a logical clock; 3 seeds.
90113

91114
## Recommended direction
92115

93-
The fix for #2617 at the CK grain is to score `ckIndex` by a fair discipline
94-
(SFQ/stride virtual time, or DRR) instead of by head timestamp, advancing the
95-
per-key state inside the Lua that maintains `ckIndex`. Both spikes agree on the
96-
discipline; this one shows it works at the real seam. Next step past the spike is
97-
a design for holding per-key virtual-time state in Redis and advancing it in the
98-
enqueue/dequeue Lua, plus the multi-shard/multi-consumer story neither spike
99-
covers.
116+
Score `ckIndex` by a fair discipline (SFQ/stride virtual time, or DRR) instead of
117+
by head timestamp. Both spikes agree on the discipline and this one shows the
118+
ordering fix works through the real dequeue path at `maxCount = 1`. The design
119+
spike past this needs to: advance per-key virtual-time state inside the batched
120+
CK-dequeue Lua (the `maxCount > 1` path this spike did not exercise), hold that
121+
state in Redis for the multi-consumer case, and address the per-CK
122+
concurrency-limit multiplication that is the other half of #2617.

internal-packages/run-engine/src/run-queue/fairness-spike-ck/harness/ckScenarios.ts

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,44 @@ import type { WorkloadConfig } from "../../fairness-spike/harness/workload.js";
44
* Concurrency-key scenarios. Each "tenant" in the workload is a concurrency key
55
* under one shared base queue. All keys are equal weight (concurrency keys carry
66
* no configured weight in production). Seed is set per-run by the bench.
7+
*
8+
* Every scenario gives keys genuinely divergent head ages (a bulk key with a
9+
* same-time backlog keeps an old head; other keys arrive via poisson so their
10+
* heads are distinct and later). This is deliberate: if all runs shared one
11+
* enqueue timestamp the ckIndex scores would tie and the real Lua's
12+
* ZRANGEBYSCORE would fall back to a lexicographic member-name tie-break, which
13+
* would make the baseline look starved for reasons that have nothing to do with
14+
* age order. We want the baseline to exercise the real age dynamic #2617
15+
* describes.
716
*/
817
export const CK_SCENARIOS: Record<string, Omit<WorkloadConfig, "seed">> = {
9-
// The direct #2617 reproduction: one key with a big backlog vs many light keys.
18+
// #2617 classic: one key fires a big backlog at once (its head stays old), four
19+
// other keys trickle in. Age-order serves the old-headed backlog to exhaustion
20+
// and starves the others.
1021
ckSkew: {
1122
envConcurrencyLimit: 4,
1223
tenants: [
1324
{ tenantId: "heavy", runCount: 240, holdMsMean: 25 },
14-
{ tenantId: "light-1", runCount: 15, holdMsMean: 25 },
15-
{ tenantId: "light-2", runCount: 15, holdMsMean: 25 },
16-
{ tenantId: "light-3", runCount: 15, holdMsMean: 25 },
17-
{ tenantId: "light-4", runCount: 15, holdMsMean: 25 },
25+
{ tenantId: "light-1", runCount: 15, arrival: "poisson", ratePerSec: 10, holdMsMean: 25 },
26+
{ tenantId: "light-2", runCount: 15, arrival: "poisson", ratePerSec: 10, holdMsMean: 25 },
27+
{ tenantId: "light-3", runCount: 15, arrival: "poisson", ratePerSec: 10, holdMsMean: 25 },
28+
{ tenantId: "light-4", runCount: 15, arrival: "poisson", ratePerSec: 10, holdMsMean: 25 },
1829
],
1930
},
2031

32+
// Symmetric keys, all arriving via poisson at the same rate. Baseline should be
33+
// roughly fair here (no key is systematically older), which is the sanity check.
2134
ckBalanced: {
2235
envConcurrencyLimit: 4,
2336
tenants: [
24-
{ tenantId: "k-a", runCount: 60, holdMsMean: 25 },
25-
{ tenantId: "k-b", runCount: 60, holdMsMean: 25 },
26-
{ tenantId: "k-c", runCount: 60, holdMsMean: 25 },
27-
{ tenantId: "k-d", runCount: 60, holdMsMean: 25 },
37+
{ tenantId: "k-a", runCount: 60, arrival: "poisson", ratePerSec: 20, holdMsMean: 25 },
38+
{ tenantId: "k-b", runCount: 60, arrival: "poisson", ratePerSec: 20, holdMsMean: 25 },
39+
{ tenantId: "k-c", runCount: 60, arrival: "poisson", ratePerSec: 20, holdMsMean: 25 },
40+
{ tenantId: "k-d", runCount: 60, arrival: "poisson", ratePerSec: 20, holdMsMean: 25 },
2841
],
2942
},
3043

31-
// A bulk key plus keys whose runs trickle in, to exercise wait and CoDel.
44+
// A bulk backlog plus two keys trickling in slowly, to exercise wait and CoDel.
3245
ckTrickle: {
3346
envConcurrencyLimit: 4,
3447
tenants: [

0 commit comments

Comments
 (0)