Skip to content

Commit 6ab36c4

Browse files
committed
test(run-engine): address inquisition (metric, floor, honesty)
Second review round (blind multi-model) found real issues: - Contention metric counted poisson tenants as contending before their runs arrived, so trickle numbers measured arrival shape not fairness; it now only counts a tenant once it has arrived, unserved work. - SFQ/stride floor was not monotonic, so a returning idle tenant could monopolise service (the CFS min_vruntime guarantee the comments claimed but did not implement). Floor is now monotonic non-decreasing. - Dropped the misleading EEVDF eligibility term from SFQ (it never changed the ordering); it is plain start-time WFQ. - FINDINGS corrected throughout: the DRR shortfall is a batch-drain measurement artifact (and virtual-time clusters queues at ties too, so the earlier explanation was wrong); worstWaitP99 is not an anti-staleness signal; the baseline age bias is not exercised here; cost was not rigorously measured; CoDel actively hurts under trickle arrival. Added the definitional-advantage and age-bias caveats up front. - Persist rough cost proxies to results JSON; note flushdb scope; drop dead seed field from scenarios.
1 parent b19f4ef commit 6ab36c4

14 files changed

Lines changed: 685 additions & 256 deletions

File tree

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

Lines changed: 141 additions & 119 deletions
Large diffs are not rendered by default.

internal-packages/run-engine/src/run-queue/fairness-spike/fairnessSpike.bench.test.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,18 @@ describe("fairness spike bench", () => {
143143
const runs = runsBySelector.get(name)!;
144144
const cwsw = stats(runs.map((r) => r.metrics.contentionWorstShareOverWeight));
145145
const jain = stats(runs.map((r) => r.metrics.contentionJain));
146-
const lightWaitP99 = stats(runs.map((r) => r.metrics.worstWaitP99));
146+
const worstWaitP99 = stats(runs.map((r) => r.metrics.worstWaitP99));
147147
return {
148148
selector: name,
149149
contentionWorstShareOverWeight: cwsw,
150150
contentionJain: jain,
151-
worstWaitP99: lightWaitP99,
151+
worstWaitP99: worstWaitP99,
152+
// Rough cost proxies. redisOps here is the number of strategy
153+
// invocations, which is NOT comparable across selectors (a candidate
154+
// reads all queues per call; the baseline short-circuits when the env
155+
// is at capacity), so treat as illustrative only.
156+
selectionRounds: stats(runs.map((r) => r.metrics.redisOps)),
157+
wallClockMs: stats(runs.map((r) => r.metrics.wallClockMs)),
152158
detailSeed0: runs[0].metrics.perGroup as GroupMetrics[],
153159
};
154160
});

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ export async function runScenario(config: DriverConfig): Promise<RunMetrics> {
5252
const env = authenticatedEnv(limit);
5353

5454
const admin = createRedisClient(config.redis);
55+
// NOTE: flushes the whole Redis DB, ignoring keyPrefix. Safe against the
56+
// dedicated testcontainer this spike runs on; do not point config.redis at a
57+
// shared instance.
5558
await admin.flushdb();
5659

5760
const queue = new RunQueue({

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

Lines changed: 52 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,12 @@ export type RunMetrics = {
3333
*/
3434
contentionWorstShareOverWeight: number;
3535
contentionJain: number;
36-
/** anti-staleness tail: the largest per-group p99 wait */
36+
/**
37+
* The largest per-group p99/max wait. NOTE: this is NOT an anti-staleness win
38+
* signal. It is dominated by the highest-volume tenant, which a fair selector
39+
* deliberately makes wait longer, so a fairer selector can score WORSE here.
40+
* Read per-tenant waits (perGroup) for the anti-staleness story instead.
41+
*/
3742
worstWaitP99: number;
3843
worstWaitMax: number;
3944
};
@@ -53,30 +58,57 @@ function jain(values: number[]): number {
5358
}
5459

5560
/**
56-
* Counts each group's dequeues that fall within the contention window: the
57-
* prefix of the dequeue timeline during which at least two groups still have
58-
* unfinished work. Once only one group has work left there is no contention to
59-
* be fair about, so those dequeues are excluded.
61+
* Counts each group's dequeues that happen during genuine contention: a dequeue
62+
* counts only if, at that instant, at least two groups have work that has
63+
* already arrived (been enqueued) but not yet been served. This excludes both
64+
* the drain tail (one group left) and, crucially, any stretch where a group's
65+
* runs have not arrived yet under spread (poisson) arrival: a tenant only
66+
* "contends" once its work exists, otherwise the metric would penalise it for
67+
* the arrival process throttling its supply rather than the selector starving it.
6068
*/
61-
function contentionCounts(
62-
events: DequeueEvent[],
63-
totals: Record<GroupId, number>
64-
): { counts: Map<GroupId, number>; windowSize: number; contended: Set<GroupId> } {
69+
function contentionCounts(events: DequeueEvent[]): {
70+
counts: Map<GroupId, number>;
71+
windowSize: number;
72+
contended: Set<GroupId>;
73+
} {
6574
const ordered = [...events].sort((a, b) => a.dequeueAtMs - b.dequeueAtMs);
66-
const remaining = new Map<GroupId, number>(Object.entries(totals));
75+
76+
// Per-group sorted arrival (enqueue) times, so we can ask how many of a
77+
// group's runs have arrived by a given instant.
78+
const arrivalsByGroup = new Map<GroupId, number[]>();
79+
for (const e of events) {
80+
const arr = arrivalsByGroup.get(e.groupId) ?? [];
81+
arr.push(e.enqueueAtMs);
82+
arrivalsByGroup.set(e.groupId, arr);
83+
}
84+
for (const arr of arrivalsByGroup.values()) arr.sort((a, b) => a - b);
85+
86+
const arrivedBy = (g: GroupId, t: number): number => {
87+
const arr = arrivalsByGroup.get(g) ?? [];
88+
let n = 0;
89+
for (const a of arr) {
90+
if (a <= t) n++;
91+
else break;
92+
}
93+
return n;
94+
};
95+
96+
const dequeuedSoFar = new Map<GroupId, number>();
6797
const counts = new Map<GroupId, number>();
6898
const contended = new Set<GroupId>();
6999
let windowSize = 0;
70100

71101
for (const e of ordered) {
72-
const withWork = [...remaining.entries()].filter(([, n]) => n > 0);
73-
if (withWork.length < 2) break;
74-
// Every group that still has work during this step is contending, whether
75-
// or not it is the one being served (so a starved group counts as share 0).
76-
for (const [g] of withWork) contended.add(g);
77-
counts.set(e.groupId, (counts.get(e.groupId) ?? 0) + 1);
78-
windowSize++;
79-
remaining.set(e.groupId, (remaining.get(e.groupId) ?? 0) - 1);
102+
const t = e.dequeueAtMs;
103+
const withWork = [...arrivalsByGroup.keys()].filter(
104+
(g) => arrivedBy(g, t) - (dequeuedSoFar.get(g) ?? 0) > 0
105+
);
106+
if (withWork.length >= 2) {
107+
for (const g of withWork) contended.add(g);
108+
counts.set(e.groupId, (counts.get(e.groupId) ?? 0) + 1);
109+
windowSize++;
110+
}
111+
dequeuedSoFar.set(e.groupId, (dequeuedSoFar.get(e.groupId) ?? 0) + 1);
80112
}
81113

82114
return { counts, windowSize, contended };
@@ -89,12 +121,11 @@ export function computeMetrics(input: {
89121
redisOps: number;
90122
wallClockMs: number;
91123
}): RunMetrics {
92-
const { events, weights, totals } = input;
124+
const { events, weights } = input;
93125
const groupIds = Object.keys(weights);
94126
const total = events.length;
95-
const sumWeights = groupIds.reduce((a, g) => a + (weights[g] ?? 1), 0);
96127

97-
const { counts: windowCounts, windowSize, contended } = contentionCounts(events, totals);
128+
const { counts: windowCounts, windowSize, contended } = contentionCounts(events);
98129
const sumWindowWeights = [...contended].reduce((a, g) => a + (weights[g] ?? 1), 0);
99130

100131
const waitsByGroup = new Map<GroupId, number[]>();

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

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,8 @@ import type { WorkloadConfig } from "./workload.js";
66
* scenario gives one tenant many queues, which is how the #2617 starvation
77
* (a tenant multiplying its selection chances) shows up at the base-queue grain.
88
*/
9-
export const SCENARIOS: Record<string, WorkloadConfig> = {
9+
export const SCENARIOS: Record<string, Omit<WorkloadConfig, "seed">> = {
1010
balanced: {
11-
seed: "spike-1",
1211
envConcurrencyLimit: 5,
1312
tenants: [
1413
{ tenantId: "t-a", runCount: 50, holdMsMean: 30 },
@@ -19,7 +18,6 @@ export const SCENARIOS: Record<string, WorkloadConfig> = {
1918
},
2019

2120
adversarialSkew: {
22-
seed: "spike-1",
2321
envConcurrencyLimit: 5,
2422
tenants: [
2523
{ tenantId: "heavy", runCount: 250, queueCount: 30, holdMsMean: 25 },
@@ -32,7 +30,6 @@ export const SCENARIOS: Record<string, WorkloadConfig> = {
3230
},
3331

3432
weighted: {
35-
seed: "spike-1",
3633
envConcurrencyLimit: 4,
3734
tenants: [
3835
{ tenantId: "big", runCount: 300, weight: 3, holdMsMean: 30 },
@@ -41,7 +38,6 @@ export const SCENARIOS: Record<string, WorkloadConfig> = {
4138
},
4239

4340
burst: {
44-
seed: "spike-1",
4541
envConcurrencyLimit: 6,
4642
tenants: Array.from({ length: 6 }, (_, i) => ({
4743
tenantId: `burst-${i}`,
@@ -52,7 +48,6 @@ export const SCENARIOS: Record<string, WorkloadConfig> = {
5248
},
5349

5450
longHold: {
55-
seed: "spike-1",
5651
envConcurrencyLimit: 4,
5752
tenants: [
5853
{ tenantId: "slow-1", runCount: 40, holdMsMean: 500 },
@@ -66,7 +61,6 @@ export const SCENARIOS: Record<string, WorkloadConfig> = {
6661
// whose runs then wait. This makes the baseline's age bias live and gives a
6762
// CoDel wrapper divergent per-tenant sojourns to react to.
6863
trickleStale: {
69-
seed: "spike-1",
7064
envConcurrencyLimit: 4,
7165
tenants: [
7266
{ tenantId: "heavy", runCount: 300, queueCount: 20, holdMsMean: 25 },

internal-packages/run-engine/src/run-queue/fairness-spike/results/adversarialSkew.json

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,16 @@
3131
"min": 1653,
3232
"max": 1989
3333
},
34+
"selectionRounds": {
35+
"mean": 646,
36+
"min": 638,
37+
"max": 658
38+
},
39+
"wallClockMs": {
40+
"mean": 5439,
41+
"min": 5087,
42+
"max": 6100
43+
},
3444
"detailSeed0": [
3545
{
3646
"groupId": "heavy",
@@ -117,6 +127,16 @@
117127
"min": 1632,
118128
"max": 2050
119129
},
130+
"selectionRounds": {
131+
"mean": 646,
132+
"min": 636,
133+
"max": 653
134+
},
135+
"wallClockMs": {
136+
"mean": 4455.333333333333,
137+
"min": 3897,
138+
"max": 4776
139+
},
120140
"detailSeed0": [
121141
{
122142
"groupId": "heavy",
@@ -203,6 +223,16 @@
203223
"min": 1626,
204224
"max": 2051
205225
},
226+
"selectionRounds": {
227+
"mean": 649.6666666666666,
228+
"min": 639,
229+
"max": 657
230+
},
231+
"wallClockMs": {
232+
"mean": 4385.333333333333,
233+
"min": 4233,
234+
"max": 4534
235+
},
206236
"detailSeed0": [
207237
{
208238
"groupId": "heavy",
@@ -289,6 +319,16 @@
289319
"min": 1632,
290320
"max": 2050
291321
},
322+
"selectionRounds": {
323+
"mean": 646,
324+
"min": 636,
325+
"max": 653
326+
},
327+
"wallClockMs": {
328+
"mean": 4208,
329+
"min": 4118,
330+
"max": 4357
331+
},
292332
"detailSeed0": [
293333
{
294334
"groupId": "heavy",
@@ -375,6 +415,16 @@
375415
"min": 1632,
376416
"max": 2050
377417
},
418+
"selectionRounds": {
419+
"mean": 646,
420+
"min": 636,
421+
"max": 653
422+
},
423+
"wallClockMs": {
424+
"mean": 4296.666666666667,
425+
"min": 3828,
426+
"max": 4587
427+
},
378428
"detailSeed0": [
379429
{
380430
"groupId": "heavy",
@@ -461,6 +511,16 @@
461511
"min": 1653,
462512
"max": 1989
463513
},
514+
"selectionRounds": {
515+
"mean": 646,
516+
"min": 638,
517+
"max": 658
518+
},
519+
"wallClockMs": {
520+
"mean": 5449.333333333333,
521+
"min": 5333,
522+
"max": 5508
523+
},
464524
"detailSeed0": [
465525
{
466526
"groupId": "heavy",

internal-packages/run-engine/src/run-queue/fairness-spike/results/balanced.json

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,16 @@
2929
"min": 1126,
3030
"max": 1283
3131
},
32+
"selectionRounds": {
33+
"mean": 370.3333333333333,
34+
"min": 365,
35+
"max": 377
36+
},
37+
"wallClockMs": {
38+
"mean": 930.3333333333334,
39+
"min": 783,
40+
"max": 1196
41+
},
3242
"detailSeed0": [
3343
{
3444
"groupId": "t-a",
@@ -93,6 +103,16 @@
93103
"min": 1109,
94104
"max": 1271
95105
},
106+
"selectionRounds": {
107+
"mean": 366.3333333333333,
108+
"min": 357,
109+
"max": 373
110+
},
111+
"wallClockMs": {
112+
"mean": 734.3333333333334,
113+
"min": 576,
114+
"max": 874
115+
},
96116
"detailSeed0": [
97117
{
98118
"groupId": "t-a",
@@ -157,6 +177,16 @@
157177
"min": 1120,
158178
"max": 1279
159179
},
180+
"selectionRounds": {
181+
"mean": 364.6666666666667,
182+
"min": 359,
183+
"max": 368
184+
},
185+
"wallClockMs": {
186+
"mean": 772,
187+
"min": 550,
188+
"max": 925
189+
},
160190
"detailSeed0": [
161191
{
162192
"groupId": "t-a",
@@ -221,6 +251,16 @@
221251
"min": 1109,
222252
"max": 1271
223253
},
254+
"selectionRounds": {
255+
"mean": 366.3333333333333,
256+
"min": 357,
257+
"max": 373
258+
},
259+
"wallClockMs": {
260+
"mean": 757,
261+
"min": 721,
262+
"max": 819
263+
},
224264
"detailSeed0": [
225265
{
226266
"groupId": "t-a",
@@ -285,6 +325,16 @@
285325
"min": 1109,
286326
"max": 1271
287327
},
328+
"selectionRounds": {
329+
"mean": 366.3333333333333,
330+
"min": 357,
331+
"max": 373
332+
},
333+
"wallClockMs": {
334+
"mean": 807.6666666666666,
335+
"min": 721,
336+
"max": 871
337+
},
288338
"detailSeed0": [
289339
{
290340
"groupId": "t-a",
@@ -349,6 +399,16 @@
349399
"min": 1126,
350400
"max": 1283
351401
},
402+
"selectionRounds": {
403+
"mean": 370.3333333333333,
404+
"min": 365,
405+
"max": 377
406+
},
407+
"wallClockMs": {
408+
"mean": 883,
409+
"min": 793,
410+
"max": 1031
411+
},
352412
"detailSeed0": [
353413
{
354414
"groupId": "t-a",

0 commit comments

Comments
 (0)