Skip to content

Commit 4e8274d

Browse files
committed
docs(run-engine): record CK vtime known limitations for GA decision
Captures the whole-branch review findings deliberately not code-fixed (bounded / self-healing / pre-existing): ckVtime tombstone drift on ack/TTL/DLQ/rollback paths and its 24h-TTL / key-delete mitigation; the pre-existing member-name tie-break among equal tags; future-scheduled variants occupying the pass-1 window under retry storms; and the rollout/rollback sequence.
1 parent de774ca commit 4e8274d

1 file changed

Lines changed: 70 additions & 0 deletions

File tree

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
# CK virtual-time scheduling: known limitations (read before enabling)
2+
3+
The feature ships behind `RUN_ENGINE_CK_VTIME_SCHEDULING_ENABLED` (off by default).
4+
A three-model blind adversarial review found no Critical issues; the correctness
5+
and safety fixes it surfaced are applied. The items below are the review findings
6+
that were deliberately NOT code-fixed because they are bounded, self-healing, or
7+
pre-existing. They are the checklist for the "enable in production" decision.
8+
9+
## Bounded state drift on paths that don't GC `ckVtime`
10+
11+
The vtime dequeue command GCs a drained variant from both `ckIndex` and `ckVtime`.
12+
But `acknowledgeMessageCkTracked`, `expireTtlRuns`, `moveToDeadLetterQueueCkTracked`,
13+
and the flag-off dequeue command do NOT remove a drained variant from `ckVtime`
14+
(they were left byte-identical). Consequences, all bounded:
15+
16+
- A low-tag tombstone (a variant emptied by ack/TTL/DLQ without a vtime serve) is
17+
the minimum entry, so the very next vtime dequeue visits it first, finds the
18+
queue empty, and GCs it: self-heals in ~1 call. It can pin the floor low for
19+
that one call.
20+
- A high-tag tombstone (a heavily-served variant whose remaining backlog is then
21+
removed out-of-band) lingers until the floor climbs to its tag or the 24h state
22+
TTL fires. Pure memory drift, does not affect fairness.
23+
- Rollback (flag on -> off): variants drained by the old command leave inert
24+
`ckVtime` entries. Old code never reads them; they expire within `stateTtl`
25+
(default 24h) once the base queue stops receiving writes. To reclaim sooner,
26+
delete the `*:ckVtime` / `*:ckVtimeFloor` keys after disabling.
27+
28+
A full fix (vtime-aware ack/TTL/DLQ command variants) is deferred: it adds three
29+
more command variants for a bounded, self-healing drift on a dark feature.
30+
31+
## Tie-break among equal virtual-time tags is member-name order
32+
33+
When variants tie at the same tag (a fresh batch at the floor: cold start, new
34+
deploy, or a GC'd variant re-entering), pass 1's `ZRANGE ckVtime` falls back to
35+
Redis's lexicographic member order, i.e. the fully-qualified queue name including
36+
the client-chosen concurrency key. A lex-early name gets a first-serve head start
37+
in a tie. This is PRE-EXISTING (the head-timestamp baseline ties the same way) and
38+
bounded: tags diverge after the first serve, so it affects only first-serve order,
39+
not long-run fairness. A future improvement is to tie-break by head age instead of
40+
member name. Do not rank fairness on an untrusted string if that head start ever
41+
matters at scale.
42+
43+
## Future-scheduled / retry-backoff variants occupy pass-1 window slots
44+
45+
Enqueue and nack register a variant in `ckVtime` even when its head message is
46+
scheduled in the future (delayed run, nack backoff). Pass 1 selects by tag with no
47+
readiness filter, so a burst of future-headed variants can fill the pass-1 window
48+
(`maxCount * scanWindowMultiplier`, default 3x); actual serves then come from pass
49+
2 (today's age order). Work conservation still holds (pass 2 is a superset), so
50+
this is fairness degradation under a retry storm, not loss. Widen
51+
`scanWindowMultiplier` if observed.
52+
53+
## Minor operational notes
54+
55+
- Idle-polling a CK queue whose only work is future-scheduled now does a couple of
56+
extra Redis writes per poll (floor SET + EXPIRE) vs the old early-return. Bounded;
57+
visible in Redis write metrics after enabling.
58+
- `descriptorFromQueue` positional parsing mis-splits a concurrency key containing
59+
a literal `:` (pre-existing; not introduced here). The vtime feature uses the
60+
full queue key as the ZSET member, which is unaffected, but any code that parses
61+
the member back into fields inherits the pre-existing limitation.
62+
63+
## Rollout (from the plan)
64+
65+
1. Deploy with the flag off (new command scripts registered, never called).
66+
2. Enable on a staging cell; watch dequeue-latency spans and Redis op rates against
67+
the op-count budget; run a sybil-shaped workload and confirm the light key's wait.
68+
3. Enable in production; during the instance-rolling window behaviour interpolates
69+
between age order and fair order (both endpoints safe).
70+
4. Rollback = flip the env var off; stale vtime keys expire via TTL within 24h.

0 commit comments

Comments
 (0)