Rebase Snake onto v25 - #908
Draft
bgwines wants to merge 65 commits into
Draft
Conversation
VTTablets under high load need a mechanism to shed excess requests rather than letting queue depth grow unbounded. The Python [loadshed-lock](https://github.com/quip/loadshed-lock) system implements this using the CoDel (Controlled Delay) algorithm — the same algorithm used in Linux's `fq_codel` queueing discipline — to detect persistent queue buildup and drop low-priority requests before they accumulate latency that compounds downstream. This PR ports that system to Go as "Snake" (Project Snakeskin). The core challenge is that Python's asyncio provides implicit single-threaded synchronization, while Go requires explicit synchronization under true parallelism. The design uses a single shared `sync.Mutex` across all layers to mirror Python's single-threaded guarantee, with `chan error` replacing `asyncio.Future` for per-request signaling. ``` Snake (public API — owns sync.Mutex) │ ├── Acquire(ctx, valveID) → *SafeUnlock │ ├── uncontended: grant immediately, start max-age timer │ └── contended: enqueue → select { req.result | ctx.Done() } │ ├── SafeUnlock.Release(errs...) │ ├── verify nonce (sync.Once for idempotency) │ ├── run release callbacks (mutex NOT held) │ └── dequeue current holder, signal next waiter │ ├── SelfContentionAwareCoDelQueue (valve layer) │ │ │ │ N valves (one per valve ID, typically one per HTTP request │ │ on the app server). Each valve permits at most 1 request │ │ into the CoDel queue; the rest wait in a per-ID FIFO slice. │ │ │ │ Owns drop orchestration: lockedRunScheduledDrop encapsulates │ │ finding the lowest-priority droppable request and triggering │ │ valve promotion. │ │ │ │ valve "A" valve "B" valve "C" │ │ ┌─────────┐ ┌─────────┐ ┌─────────┐ │ │ │ A₂ A₃ │ │ B₂ │ │ (empty) │ │ │ └────┬────┘ └────┬────┘ └────┬────┘ │ │ │ promote │ promote │ │ │ ▼ ▼ ▼ │ └───► CoDelQueue ◄─────────────────────────────────────── │ (single shared instance) │ ├── CoDelQueue (CoDel algorithm) │ ├── *list.List: [A₁] [B₁] [C₁] (O(1) head pop + O(1) removal) │ ├── healthy ↔ dropping state machine │ ├── control law: interval / count^exponent │ └── schedules drops via injected callback — no timer ownership │ └── Timers (owned by Snake, scheduled via callback injection) ├── dropTimer: time.AfterFunc → sq.lockedRunScheduledDrop └── maxAgeTimer: time.AfterFunc → force-release stale holder ``` | Event | Mechanism | Result channel touched? | Request leaves CoDel queue? | |---|---|---|---| | Grant | `lockedMarkNotDroppable` + `signal(nil)` | Yes — nil | No — holder stays until release | | Release | `lockedDequeue` | No (already signaled) | Yes — removed on release | | Drop | `lockedDropActive` → `signal(err)` | Yes — error | Yes — removed immediately | | Cancel | `lockedCancel` / `lockedRemove` | Yes — error | Yes — removed immediately | 1. **Synchronization**: Python relies on asyncio's single-threaded event loop (no locking). Go uses a single `sync.Mutex` on Snake guarding all state 2. **Request signaling**: Python uses `asyncio.Future`. Go uses buffered `chan error` (cap 1) paired with `signal()` that writes both an inspectable `outcome` field and the channel 3. **Timer ownership**: Both Python and Go inject scheduling into the queue. Python uses `loop.call_later()`, Go uses an injected `func(delayNs int64)` callback. Snake owns the actual `time.AfterFunc` and provides idempotency 4. **Cancel-vs-grant race**: Python has no equivalent — asyncio's cooperative scheduling prevents it. Go's Acquire uses a double-select with holder-check fallback 5. **Context cancellation**: Python cancels via `asyncio.CancelledError`. Go takes `context.Context` in Acquire with explicit `lockedCancel`/`lockedRemove` methods 6. **Release idempotency**: Python uses `__del__` fallback. Go wraps Release in `sync.Once` 7. **Max-age timeout**: Python cancels the holder's asyncio.Task. Go uses `time.AfterFunc` with staleness guard via `maxAgeHolder` pointer comparison 8. **activePerValve map**: Added for O(1) lookup of which request is in CoDel per valve ID (Python scans callbacks) 9. **Release callbacks**: Python awaits async coroutines. Go runs `func(error)` callbacks without mutex held, with `recover()` for panic safety - **`go/hack/ensure_swiss_map.go` deleted**: This file uses a build tag (`!goexperiment.swissmap`) that fails to compile on Go 1.26+ where swiss maps are always enabled and the experiment tag no longer exists. The base branch (`bwines/snake-22`) still targets Go 1.24 where this isn't an issue, but this branch builds on 1.26. The upstream fix (vitessio#19088) was already backported to upstream release-22.0 but hasn't reached our fork yet — it'll come along when snake-22 merges with slack-22.0. ~110 tests across 7 test files: - **codelq_test.go** (39 tests) — CoDel algorithm unit tests: FIFO ordering, drop priority selection, state machine transitions, control law math, scheduled drop behavior via test recorder - **selfcontentionaware_codelq_test.go** (24 tests) — Valve logic: direct entry, valving, promotion on dequeue/drop/cancel, outstanding count lifecycle, FIFO within contention, mass cancel under overload, peek cleanup - **snake_test.go** (28 tests) — Snake behavior: mutual exclusion, FIFO ordering, context cancellation, cancel-vs-grant race, self-contention serialization, CoDel drops, max-age timeout, SafeUnlock idempotency, release callbacks with panic recovery - **snake_stress_test.go** (16 tests) — High-concurrency stress tests with -race: 200 goroutines, rapid acquire/release, cancel-vs-grant races, drop timer + cancel races, max-age under load, goroutine leak detection, starvation prevention, self-contention stress - **snake_race_test.go** (1 test) — 50k-iteration targeted reproduction of the cancel-vs-grant race - **snake_overload_test.go** (12 tests) — Overload recovery, memory steady-state, CoDel state transitions, mass cancel, drop-only-droppable, max-age vs cancel race, no-panic invariant - **snake_bench_test.go** — Comprehensive benchmarks (see `BENCHMARK_RESULTS.md`) All tests pass with `-race -count=5`. --- AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
- elem → codelqElem (disambiguates from other list memberships) - *float64 → float64 (nil never occurs in practice) - lockedMarkNotDroppable → lockedOnGrant (event-based naming) - lockedDropActive → lockedDrop (remove redundant qualifier) - releaseInternal → releaseOnCancel (clarifies the call site) AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com>
### Background / Why?
Snake currently operates as a single-holder mutex (capacity=1). This works as a proof-of-concept but doesn't match the real deployment targets: vttablet's connection pool has dozens of connections, and the SCHED_IDLE dispatch gate needs a concurrency band (`[min, max)`) — not a binary lock. Generalizing to an N-holder semaphore with dynamic capacity is the prerequisite for integrating Snake into any real vttablet admission path.
The CoDel queue and self-contention valve are preserved unchanged. The only structural shift is that up to `Capacity()` concurrent grants are allowed instead of exactly one, and each release triggers exactly one new grant (no thundering herd).
```
┌─────────────────────────────────────────────────────────┐
│ Snake │
│ │
Acquire(ctx, valveID) │ ┌───────────────────────────────────────────────────┐ │
─────────────────────► │ │ SelfContentionAwareCoDelQueue │ │
│ │ │ │
│ │ valve["id-A"] ─► [r5] [r6] (pending FIFO) │ │
│ │ valve["id-B"] ─► [r7] │ │
│ │ │ │
│ │ ┌─────────────────────────────────────────────┐ │ │
│ │ │ CoDelQueue (dropping / healthy) │ │ │
│ │ │ │ │ │
│ │ │ undroppable undroppable droppable droppable │ │ |
│ │ │ ┌────┐ ┌────┐ ┌────┐ ┌────┐ │ │ │
│ │ │ │ r1 │ │ r2 │ │ r3 │ ◄fw │ r4 │ │ │ │
│ │ │ └────┘ └────┘ └────┘ └────┘ │ │ │
│ │ │ id-A GRANT id-B GRANT id-A id-B │ │ │
│ │ │ │ │ │
│ │ └─────────────────────────────────────────────┘ │ │
│ └───────────────────────────────────────────────────┘ │
│ │
│ holders: { r1, r2 } capacity: N (dynamic) │
│ │
│ On Release(r1): │
│ 1. delete(holders, r1) │
│ 2. lockedComplete(r1) ← sojourn health check │
│ 3. lockedTryGrantOne() ← grant r3 (firstWaiting) │
│ │
└─────────────────────────────────────────────────────────┘
fw = firstWaiting pointer (O(1) next-ungrant lookup)
Capacity() is dynamic — driven by SCHED_IDLE sidecar in production
```
Key design decisions:
- `holders` map replaces single `holder` pointer — O(1) membership check for the ctx.Done() race resolution
- `lockedTryGrantOne` grants exactly one waiter per release event (no thundering herd)
- Fast-path grant only for requests already in the CoDel queue (valve-queued requests wait for promotion)
- `CoDelQueue` gains `firstWaiting` (O(1) pointer) and `lockedComplete` (release-time removal with sojourn-based health transition)
- `SafeUnlock.Release` is idempotent (returns nil on double-release from max-age timer race)
- `nonce`-based auth removed — `holders` map is the authority
### Benchmarks
```
BenchmarkSnake_NHolder_Throughput/Cap1 439 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Throughput/Cap2 831 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Throughput/Cap4 936 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Throughput/Cap8 607 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Throughput/Cap16 666 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Uncontended/Cap1 227 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Uncontended/Cap4 231 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Uncontended/Cap16 230 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Saturated/Cap1 445 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Saturated/Cap4 899 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Saturated/Cap8 978 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_WithValve/Cap1 579 ns/op 431 B/op 7 allocs/op
BenchmarkSnake_NHolder_WithValve/Cap4 1069 ns/op 431 B/op 7 allocs/op
BenchmarkSnake_NHolder_WithValve/Cap8 1076 ns/op 428 B/op 7 allocs/op
BenchmarkSnake_NHolder_Allocs/Cap1 235 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Allocs/Cap4 233 ns/op 416 B/op 7 allocs/op
BenchmarkSnake_NHolder_Allocs/Cap16 231 ns/op 416 B/op 7 allocs/op
```
Uncontended fast path is ~230ns regardless of capacity (no extra overhead from generalization). Allocation count is constant at 7 per acquire/release cycle across all capacities.
### Testing
- New `snake_nholder_test.go`: concurrent grants, capacity blocking, parallel throughput invariant (max concurrent never exceeds capacity), dynamic capacity increase/decrease, valve serialization with multiple slots, NGranted accuracy under concurrency, context cancel slot freeing, max-age per-slot, memory cleanup
- All existing tests updated to use `lockedFirstWaiting` + `lockedOnGrant` + `lockedComplete` lifecycle (replaces `lockedDequeue`)
- `go test -race -count=3 ./go/vt/vttablet/tabletserver/loadshed/...` passes clean
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
### Background / Why?
The valve's gate condition in `lockedEnqueue` used `outstandingCounts[valveID] > 1` to decide whether to valve a new arrival. This is a proxy for "there's already a droppable representative in the CoDel queue", but it's wrong after a grant. When a request is granted, `lockedOnGrant` deletes `droppablePerValve[valveID]` (the droppable became undroppable), but `outstandingCounts` remains elevated because the granted request is still outstanding. Subsequent arrivals for the same valve ID see count > 1 and get valved, but there's no droppable representative to ever trigger promotion. As a result, the invariant ("each nonempty valve always has exactly one droppable entry in the CoDel queue") is broken and requests are stranded indefinitely.
This was correct in the Python implementation because:
1. it was a lock, not a semaphore
2. the invariant was slightly different (each nonempty valve has one representative in the codelq, not one DROPPABLE representative). As we noted at the time of coming up with the N-holder invariant, that invariant is actually superior even for the 1-holder (lock) case, it just only mattered in an extreme edge case we likely never saw.
The practical impact: with capacity N and M concurrent requests from the same valve ID where M ≤ N, only the first request is ever granted. The rest are serialized one-at-a-time through release→promote→grant chains, leaving N-1 capacity slots unused. This case probably wouldn't ever happen in production since it requires all requests to be for only one valve ID.
The fix replaces the `outstandingCounts > 1` check with a direct `droppablePerValve[valveID]` existence check, which directly encodes the invariant.
### Testing
new unit tests
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
* Gate OLTP read pool with Snake load shedder Wire the CoDel-based Snake gate into QueryExecutor.getConn() so that OLTP read requests are admitted or shed before consuming a MySQL connection. Controlled by --snake-enabled flag (off by default). ContentionID is extracted from the unique_id SQL margin comment that webapp injects, with a random 16-char hex fallback for requests without one. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> * Use proto UniqueId as Snake contentionID, bypass valve when unset Replace the extractUniqueID regex (which targeted a comment format webapp never sends) with the proto-based approach: read qre.options.GetUniqueId() and only enter the Snake gate when it's non-empty. Requests without a unique_id bypass the gate entirely rather than sharing a single valve slot or using random IDs. Add `string unique_id = 22` to ExecuteOptions in proto/query.proto. This is an optional field — callers that don't set it get zero-cost bypass. The sqlparser/vtgate propagation to populate it from SQL directives is deferred to a follow-up PR. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> * Gate DML with Snake load shedder (#871) Acquire a Snake slot in TxPool.Begin (for fresh connections only, not reserved conns) and release it in txComplete. This gates both autocommit and explicit transaction DML through the same CoDel-based load-shedding mechanism already used for the OLTP read pool. Requests without a unique_id bypass the gate entirely, matching the OLTP-read behavior. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> --------- Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Expose a point-in-time snapshot of CoDel internal state (queue length, droppable count, holder count, dropping state, drop count, current interval). Useful for bench harnesses and future metrics export. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The original CoDel paper uses exponent 1.0 (interval / sqrt(count)). This matches the standard algorithm behavior where drop rate scales linearly with throughput change. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Add Snake bench suite and plotting scripts Standalone bench harness (go run bench_suite.go) that exercises Snake under a linear ramp load profile, polling Stats() every 10ms. Outputs TSV data to ~/snake-load-test-charts/tsv/<date>/<timestamp>/. Two Python plotting scripts consume the TSVs: - plot_linear_ramp.py: 7-row chart (grant rate, issued/shed Hz, queue depth, CoDel state, interval, unfilled slots, latency) - plot_easing_comparison.py: 6-column × 7-row comparison across easing divisor values AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Add CLI flags for CoDel params and README Avoids config drift between bench suite and production by making exponent, target, and interval configurable via flags (defaulting to production values). Adds README explaining how to run benchmarks and plot results. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Include HH-MM-SS in plot output filenames Prevents overwrites when running benchmarks multiple times per day. Also fixes stale EXPONENT=0.5 default in plot_linear_ramp.py to match the production value of 1.0. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Make plot_linear_ramp.py config params CLI flags Replaces hardcoded CAPACITY, WORK_MS, TARGET_MS, INTERVAL_MS, EXPONENT, PEAK_MULT with argparse flags so the chart title stays accurate without manual code edits. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The cap prevented the drop timer from catching up after a burst or GC pause — if more than 100 drops were overdue, the remainder were deferred to the next timer fire. Under sustained overload this meant the queue couldn't drain fast enough, keeping the system in a continuous dropping state rather than shedding decisively and recovering. Without the cap the loop drains all overdue drops in one pass, producing a healthy oscillation: shed burst → droppableLen hits 0 → exit dropping → new arrivals refill → re-enter dropping. Benchmarks confirm the sawtooth is driven by this change alone (control law floor is irrelevant at this overload level). The original concern (CPU burn from a late timer fire) is mitigated by the fact that each iteration does O(n) work in findLowestPriorityDroppable which bounds the realistic iteration count — and the queue length itself is bounded by arrival rate × interval. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
The previous floor of 100ns was arbitrarily chosen. Since the interval is int64 nanoseconds, there's no floating point precision concern — a floor of 1ns is sufficient to ensure lockedControlLaw always makes forward progress. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
* Wire ExecuteOptions priority into Snake load shedding Snake's Acquire now takes an explicit priority in its own convention (lower priority shed first, higher shed last). The two tabletserver entrypoints translate the Vitess ExecuteOptions proto priority (0 = most important) into that convention via MaxPriorityValue - protoPriority, so loadshed stays free of any sqlparser dependency and holds no proto knowledge. The proto-priority parse-and-default logic is extracted from getPriorityFromOptions into a package-level priorityFromOptions function so TxPool, which cannot reach a TabletServer, can share it. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> * Tighten priority docstrings and rename a test Shorten the Acquire, Snake.priority, and priorityFromOptions docstrings, and rename TestSnake_Priority_HonorsCallerKey to use the package's "priority" terminology rather than "key". AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> --------- Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…t` (#887) Align the load shedder's flag surface with the Query Admission Control RFC, which specifies four flags: --loadshed-enabled, --loadshed-target, --loadshed-interval, and --loadshed-exponent. Renames the three existing snake-* flags and their TabletConfig fields, and promotes the CoDel control law exponent from a hardcoded 1.0 to the configurable --loadshed-exponent. Defaults are set to the RFC's recommended values (target 20ms, interval 400ms, exponent 1.0). AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* CoDel count ease-out with configurable easing divisor
Replace the hard count=1 reset on dropping→healthy transition with a
gradual ease-out: the timer continues to fire at progressively longer
intervals, halving (or dividing by EasingDivisor) the count each time
until it reaches 1. This eliminates oscillation where the system rapidly
bounces between dropping and healthy states.
Key changes:
- Fix lockedCurrentInterval() to compress intervals whenever count > 1
(not just when dropping), so the ease-out timer fires at correct rates
- Add EasingDivisor config param (defaults to 2.0 if nil)
- Three CoDel states: idle (count=1, no timer), easing (!dropping,
count>1, timer armed), dropping (dropping=true, timer armed)
- lockedComplete() no longer stops/restarts the timer — it just sets
dropping=false and lets the already-armed timer continue into easing
- Add Stats() method and snakeserver bench suite with chart generation
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* Add snakeserver bench suite and chart generation
Standalone load test harness (//go:build ignore) that exercises Snake
under various traffic profiles (sine, constant, linear ramp) and emits
TSV data + a Python chart generator for visualizing CoDel internals.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* Fix easing re-entry bug in lockedEnterDroppingState
When CoDel re-enters the dropping state from easing (count > 1), the
delta heuristic (count - lastCount) produces a negative number because
count was halved below lastCount during easing. This caused count to
unconditionally reset to 1, losing all dropping intensity and producing
wild oscillation in the control loop.
Fix: when count > 1, preserve it directly and skip the delta heuristic,
which is only meaningful for the idle→dropping transition (count == 1).
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* bench_suite: add -easing flag and per-request latency tracking
Avoids source edits when running comparison sweeps across easing divisor
values. Also tracks end-to-end request latency (acquire→release) so
charts can show p50/p95/p99 alongside throughput metrics.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* Easing implementation + benchmark tooling improvements
- Rename BENCHMARK_RESULTS.md → benchmark/MICROBENCHMARK_RESULTS.md
- Update plot_easing_comparison.py: fractional divisors (2^(1/2), 2^(1/3),
2^(1/4)), base branch column, --run flag, stable dir naming, stale-data
fix (always read from tsv/ subdir), remove shared y-axis normalization,
"no decay" label for divisor=1
- Add EasingDivisor to bench_suite.go config
- Update benchmark README with new workflow
- Add *.~undo-tree~ to .gitignore
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Rename snakeserver/ to benchmark/
Consolidates all bench tooling under the benchmark/ directory. The
snakeserver bench_suite.go (full-featured: parallel execution, -easing
flag, -filter flag, sine/constant/linear profiles) replaces the simpler
version that was in benchmark/. Also moves the HTTP server (main.go),
go module, and plot_suite.py.
Updates plot_easing_comparison.py --bench-go default path to reference
the local bench_suite.go now that it lives in the same directory.
Removes the accidentally-tracked snakeserver binary.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Remove LastDropsPerRun, default easing to 1.2, clean up comments
- Remove LastDropsPerRun field and all tracking state (unnecessary complexity)
- Change default EasingDivisor in tests from 2.0 to 1.2
- Condense stream-of-consciousness comment in codelq_test.go to 2 lines
- Replace "halves" with "decays" in comments (divisor is configurable, not always 2)
- Restore CurrentInterval comment to just "// ns"
- Add bench_suite binary to .gitignore
- Remove drops-per-run panel from plot_suite.py (4→3 stat panels)
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Update benchmark README to match current flags and paths
The README had stale references to --exponent/--target-ms/--interval-ms
flags that no longer exist, and was missing -j/-filter/-easing. Also
fixed the --bench-go default path (was ../snakeserver/, now local).
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* codelq: add subtract easing mode and seed dropNextNs on dropping entry
Add a configurable easing strategy to CoDelConfig: EasingMode selects
"subtract" (decrement count by EasingSubtractor each easing fire) or the
default "divide" (divide count by EasingDivisor). Both floor count at 1.
Manually rewrite a bunch of claude's ugly and overly complex code. In
doing so, also make a subtle but important fix. Previously, the easing
was being done by adding the interval to q.nowNs() instead of applying
a catchup loop based on the previous value of q.dropNextNs. As a result,
it would only ever ease out once per iteration, whose min time was
bounded by timer accuracy, making it more closely simulate a slower
easing under aggressive load. "subtract" easing with a value of 1 or 2
shows much closer performance to what we saw prior.
Dropped the count memory because it probably wasn't doing much and
we've had some very good looking results with easing.
* benchmark: configurable workloads, easing strategies, and comparison axes
Add a JSON config (--config) to plot_easing_comparison.py describing the
easings and workloads to run. Each easing entry is a divisor or a
subtract spec; "compare" selects whether figure columns vary by easing
(one figure per workload) or by workload (one figure per profile, e.g.
an interval:target sweep at a fixed easing).
bench_suite.go gains a custom single-workload mode (-profile and friends)
that replaces the preset matrix when set, plus -easing-mode/-easing-sub
to drive the subtract strategy, and a linear_ramp_down profile mirroring
linear_ramp.
Use a repo-root-relative replace path in go.mod so the bench builds from
any checkout. Add example and working configs; document the new flags.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
* touch up documentation a tiny bit
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* codelq: nil-guard EasingDivisor in lockedEaseCount
lockedEaseCount nil-guarded EasingMode and EasingSubtractor but called
q.cfg.EasingDivisor() unguarded. A CoDelConfig that omits EasingDivisor
(e.g. TestTxPoolSnake_CapacityExhaustedShedsLoad) thus segfaults when the
drop timer fires during the easing phase, crashing the whole tabletserver
test binary. Default to 2.0 when unset, matching the prior behavior.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* codelq: make logbase the only easing strategy
Collapse the configurable easing strategies (divide / subtract / sqrt-ln
log / logbase) down to a single logbase decay:
count -= floor(log_base(count) / base), floored at 1. CoDelConfig now
exposes only EasingLogBase (defaulting to 3 when unset or <= 1); the
EasingMode, EasingDivisor, and EasingSubtractor fields are removed.
Benchmark sweeps across sine, ramp, and brown-noise workloads showed the
logbase strategy gives a consistent, large median-latency reduction for a
negligible throughput cost, with the base acting as a clean
aggressiveness knob, so the other strategies are no longer needed.
Update the easing tests to the logbase semantics and rename the
EasingDivisor config fields to EasingLogBase.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
* benchmark: logbase-only easing plus new workload profiles and knobs
Easing simplification (follows the codelq change): drop the
-easing / -easing-mode / -easing-sub flags, keep -easing-log-base
(now defaulting to 3), and remove the divide/subtract/log handling in
plot_easing_comparison.py. Easing config entries are now a bare base
number or {"base": N}, and --config is required.
Workload additions accumulated while tuning the easing:
- brown_noise profile: repeatable seeded random-walk load
(-brown-seed / -brown-step / -brown-sample-ms)
- sine trough floor (-sine-floor) so a sine can swing between two
non-zero load levels
- linear_ramp_down profile mirroring linear_ramp
- plot_easing_comparison.py: --jobs/-j to cap concurrent bench
processes (use 1 for low-noise serial runs), a "compare" axis to vary
columns by easing or by workload, and a log-scale latency panel
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
* Drop stale EasingDivisor from CoDel callers
The "make logbase the only easing strategy" commit renamed the
CoDelConfig.EasingDivisor field to EasingLogBase inside the loadshed
package, but missed the two callers a directory up in query_engine.go
and tx_engine.go, so the tabletserver package no longer compiled. The
old literal (1.2) was a divide-mode divisor anyway; under the logbase
strategy the documented default base of 3 (applied when EasingLogBase is
unset) is the right behavior, so just drop the stale field.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* Pass priority arg to Acquire in benchmark module
The benchmark module has its own go.mod, so its callers weren't caught
when ExecuteOptions priority wiring (#884) added a third priority arg to
Snake.Acquire. Both call sites (bench_suite.go, main.go) still used the
2-arg signature and failed to build. Pass 0 (uniform priority), matching
the in-tree loadshed benchmarks.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
---------
Signed-off-by: Brett Wines <bwines@slack-corp.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: Ross Cohen <rcohen@slack-corp.com>
Co-authored-by: Claude <svc-devxp-claude@slack-corp.com>
* Characterize global-lock throughput under high contention Add BenchmarkSnake_HighContention to sweep Snake's single global mutex from 8 up to ~524288 concurrent contenders across the fast-path (immediate-grant), valve, and default capacity-1 serialized paths, measuring whether ns/op stays flat or collapses super-linearly. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Fold high-contention benchmark into snake_bench_test.go Move BenchmarkSnake_HighContention and its runPooledContention/ contentionLevels helpers out of the standalone snake_contention_bench_test.go and into the canonical snake_bench_test.go, consolidating the loadshed package's benchmarks in one file. Pure relocation; benchmark logic is unchanged. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> --------- Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Brett Wines <bwines@slack-corp.com>
The commit that flipped --loadshed-enabled to default true updated the flag golden files but left defaultConfig.LoadshedEnabled implicitly false, so NewDefaultConfig() disagreed with the flag-populated currentConfig and TestFlags failed (LoadshedEnabled: false vs true). Set it explicitly in defaultConfig so the two agree. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
After --loadshed-enabled flipped to default true, newTestTabletServer (which starts from NewDefaultConfig) began building tablets with Snake enabled, so TestGetConnSnakeDisabled's "snake should be nil" assertion failed. Explicitly disable loadshed in the shared helper, restoring the prior default; tests that want Snake on (TestGetConnWithSnake) already build their own config with LoadshedEnabled=true. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Adds a PublishStats helper in the loadshed package that registers one GaugeFunc per SnakeStats field (queue length, droppable length, holder count, dropping state, drop count, current interval) through the tablet Exporter. Called once per Snake instance from the query and tx engine init blocks, with SnakeOltpRead / SnakeDml name prefixes so the two pools' metrics don't collide on the Exporter's single label dimension. These ride the existing vttablet Prometheus scrape, making Snake's internal CoDel state observable in Grafana during load tests. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Snake now tracks how many requests it has shed in an internal atomic, incremented in acquireError() — the single funnel for gate-driven drops. Context cancellations return ctx.Err() without passing through that funnel, so they are correctly excluded (the caller gave up; the gate did not shed). PublishStats exports it as a per-pool CounterFunc (SnakeOltpReadShedCount / SnakeDmlShedCount). This is the monotonic count of rejected requests — the client-facing "how much did we shed" signal a load test needs for shed-rate. It is distinct from the CoDel DropCount gauge, which is non-monotonic control- law state (it rises while dropping and decays during easing) and cannot answer "how many requests were rejected." Keeping the counter inside Snake leaves tabletenv.Stats and the query/tx callers untouched. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Both Snake call sites (oltp-read getConn and dml Begin) only acquired a gate slot when the request carried a non-empty UniqueId, falling through to the bare connection pool otherwise. That silently excluded all unkeyed traffic from load shedding: a client that omits the valve ID bypassed the CoDel gate entirely. The loadshed package already handles an empty valve ID correctly — such requests enter the CoDel queue directly, bypassing only the per-valve fairness layer (per the RFC: "If the field is not present, entries bypass the valves and enter the CoDel queue directly"). The guard at the call sites contradicted that contract. Remove the `!= ""` guard at both sites so Acquire always runs when snake is non-nil, passing the possibly-empty valve ID straight through. Flip the two tests that asserted the old bypass behavior to assert the gate now engages on an empty valve ID. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…enerated protos (#893) * Rename ExecuteOptions.unique_id to loadshed_valve_id The Snake load shedder uses this ExecuteOptions field as its per-valve contention key, but the field was named unique_id while the RFC and the loadshed package both call the concept a "valve ID". The mismatch made the wiring hard to follow: callers read GetUniqueId() and passed the result into Acquire(ctx, valveID, ...). Rename the proto field unique_id -> loadshed_valve_id (field number 22 is unchanged, so the wire format is backwards-compatible) and update the two reader call sites and their tests to match. ExecuteOptions is forwarded from vtgate to vttablet wholesale, so no per-field passthrough wiring is needed. Also relocate the "empty valve ID is valid" explanation from the two call sites into Acquire's doc comment, where the contract actually lives. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> * Regenerate query protos with the repo-pinned toolchain CI's check_make_proto and check_make_vtadmin_web_proto steps were red because the committed generated files did not match what the pinned toolchain produces. Two distinct drifts, both inherited from earlier in the snake stack and unrelated to the field rename itself: - The Go .pb.go files were last generated with protoc-gen-go v1.36.11 and an off-version vtproto plugin, while the repo pins protobuf v1.36.5 (go.mod) and vtprotobuf v0.6.1-...79df5c4 (go.mod). main is already on the pinned output, so regenerating reverts the header and the CloneVT codegen pattern to match main. - The vtadmin web bindings (vtadmin.js / vtadmin.d.ts) were never regenerated when transaction_timeout, no_result, and the field at number 22 were added, so they were missing all three. Regenerating adds them, with field 22 carrying its new name loadshed_valve_id. Regenerated via `make proto` + `make vtadmin_web_proto_types` using the toolchain installed by bootstrap (protoc 3.21.3, protoc-gen-go v1.36.5, protoc-gen-go-vtproto v0.6.1-...79df5c4). AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com> --------- Signed-off-by: Brett Wines <bwines@slack-corp.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Track live pool capacity in Snake load-shedder gates The oltp-read and dml Snake gates derived their capacity from the static config fields (config.OltpReadPool.Size / config.TxPool.Size). A runtime pool resize — e.g. an operator shrinking ReadPoolSize via /debug/env — calls SetCapacity on the pool only, never writing back to config, so the gate kept admitting at the old ceiling while the smaller pool queued the overflow in its FIFO waiter list. The CoDel gate was effectively bypassed. Point each capacity closure at the pool's live Capacity() instead. Pool capacity reports 0 until the pool is opened, so fall back to the configured size during that startup window to avoid throttling the gate to one holder. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Trim Snake capacity closure comments to two lines AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
After --loadshed-enabled flipped to default true, the endtoend framework (framework.StartServer, built from NewDefaultConfig) began starting tablets with the Snake gate live. Under the race detector the e2e_race job inflates per-operation latency past CoDel's 20ms sojourn target, so the dml Snake shed legitimate test traffic and 27 transaction tests failed with RESOURCE_EXHAUSTED "dml load shed: request dropped by CoDel queue". Disable loadshed in framework.StartServer and in the three StartCustomServer callers (connkilling, connecttcp, twopc) so no e2e package carries a live gate. Mirrors the unit-test fix in #891. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Move the CoDel health check from completion (Release) to grant (dispatch), so sojourn measures pure queue-wait time rather than queue-wait plus resource hold time. Hold time is work latency, not queue backup; including it caused the controller to shed on essentially-healthy systems when work duration approached the target. The check now runs unconditionally in lockedOnGrant; lockedComplete is reduced to unlinking the request. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Add -work-stddev-ms to bench_suite: when >0, each request's work duration is drawn from a Gaussian N(work-ms, stddev) clamped >=0, seeded and mutex-guarded for repeatable, goroutine-safe runs. Wire it through the plotter config and document it. Also switch the latency plot row to a linear axis capped at 500ms for readability. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Replace the TriggerGatedDropping bool with a DropMode enum governing how a
dropping episode leaves the count==1 state:
- DropSlowStart (default): arm on enqueue, ramp count++ per drop. The
original always-arm CoDel behavior, preserved bit-for-bit.
- DropJumpStart: arm only when the oldest waiter's sojourn crosses
TriggerNs (monitored by a timer), jumping count to log2(droppableLen).
- DropBoth: arm on enqueue and ramp, but while count==1 also watch the
head's sojourn; leave count==1 by whichever fires first. The drop timer
wakes at min(ramp deadline, head trigger deadline) so neither escalation
is slept through.
Add GraceCount: a count threshold below which the head drop is suppressed
while the count ramp and timer pacing continue as usual. Defaults to 1
(disabled, since count is always >= 1). In both mode the jump window stays
open throughout the grace window and seeds max(count, log2(...)).
The jump's first paced drop is anchored at the head's trigger-crossing
deadline rather than the (possibly late) fire time, consistent across the
monitor and both-mode jump paths.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Add -drop-mode (slow|jump|both), -trigger-ms, and -grace-count flags to bench_suite, replacing the -trigger-gated bool. Thread the matching drop_mode/trigger_ms/grace_count fields through the plotter config and list all params in the comparison chart header. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
lockedFindLowestPriorityDroppable was an O(n) scan of the CoDel queue, run under s.mu inside the drop loop. For the all-priority-0 workload it early-exited at the first droppable entry, so it was cheap and invisible. But with mixed priorities the lowest is no longer at the front, so each call walks the full queue -- and under overload the drop loop calls it repeatedly while shed goroutines convoy behind the lock. Measured under constant 20x overload, CPU-starved: with mixed priorities [0,100] the old scan produced 363 HOURS of cumulative s.mu contention vs 13.6s with the index (~96,000x), with lockedFinishRelease/runDropTimer pegged at 98% on the scan. The index collapses that back to the same regime as the priority-0 case (runDropTimer share drops to ~2%). This is a robustness fix against a real production shape (priority-tagged traffic under load), not a micro-opt: the all-priority-0 benchmark shows no change because its early exit already made the scan cheap. droppableIndex keeps one FIFO bucket per integer priority in the production domain [0,100], plus a 2-word occupancy bitset so the lowest non-empty bucket is found via TrailingZeros64. Out-of-domain priorities (non-integer, out-of-range, +Inf -- exercised by existing tests) fall into an overflow list treated as the highest priority; it is empty in production and scanned only when non-empty. insert/remove/min are all O(1). Requests carry their bucket node (bucketElem/bucketIdx) for O(1) removal. The index is kept in lockstep with droppableLen: every ++/-- transition (enqueue, peek-cleanup, popElem, remove, and the droppable->undroppable flip in onGrant) pairs with an index insert/remove, so min() never returns a signaled or granted request -- preserving the old scan's exact contract: the oldest request at the lowest priority present. The benchmark harness gains -mixed-priority to issue uniform [0,100] priorities, needed to exercise (and demonstrate) this path. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com> Signed-off-by: Ross Cohen <rcohen@slack-corp.com>
MaxAge force-released a slot after a fixed duration via a per-holder time.AfterFunc. It was carried over from the original Python implementation, where it backstopped a leaked slot. In Go the entire Acquire->Get->exec->Release window is already bounded by the caller's context (qre.ctx / QueryTimeout), so a granted request cannot outlive its context -- the timer is redundant. Neither production snake (OLTP-read in query_engine.go, DML in tx_pool.go) ever set it. Remove the MaxAge config field, the maxAgeTimers map, the start/stop timer helpers, and their call sites in lockedGrant / release / releaseOnCancel. This also drops two runtime-timer operations (Stop + AfterFunc) from every grant/release, which were extending the s.mu critical section. Drop the now-dead MaxAge tests and the 30s MaxAge setting from the benchmark harness. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…quiet The sync-shed redesign moved CoDel shedding onto the release/dequeue path, but the dequeue path only called lockedAdvance, which can advance an already-active episode but cannot start or re-establish one. Episode initiation (setting dropping=true) lived solely in lockedArmDropTimer, reachable only from the timer. So when lockedOnGrant tore an episode down (its health check clears dropping on a grant whose sojourn was under target), only the backstop timer re-armed it. Raising MinDropDelay to effectively disable the timer therefore stopped all shedding: once torn down, dropping stayed false and no drops fired. With the default 1ms delay the bug was masked because the timer re-armed on nearly every fire. Have the dequeue path run the full CoDel logic (new lockedDequeue), mirroring lockedRunTimer: the jump/monitor trigger checks run on every call (a head-sojourn crossing is time-based, independent of drop pacing), while the paced drop/ease/re-arm work runs only when a drop is due (now >= dropNextNs). The re-arm is what re-establishes dropping, so the dequeue path is now self-sufficient and the timer is a true backstop. Widen lockedNeedsAdvance to also fire on a droppable backlog so the trigger check gets a chance. Adds a queue-level regression test (shed under overload with the backstop timer effectively off) and fixes the prior sync-shed test, which forced an impossible armed state (dropNextNs=0) and used an inconsistent clock. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
plot_seed_comparison.py renders an N-column comparison across bench_suite runs (typically brown_noise seeds), laid out as 7 rows: grant rate, issued/granted/shed, queue depth, CoDel dropping state, CoDel interval (log), cumulative unfilled slots, and request latency percentiles (p50/ p95/p99, clamped). Reads the per-label <label>.tsv / <label>_stats.tsv pairs bench_suite writes; columns are chosen via --labels. Reconstructs the seed-sweep chart that previously existed only as a saved image. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Both Snake call sites (OLTP read in query_executor.go, DML in tx_pool.go) duplicated the proto-to-Snake priority inversion inline. Extract a single snakePriorityFromOptions helper next to priorityFromOptions so the inversion rationale lives in one place and both pools stay in lockstep. No behavior change: same default (TxThrottlerDefaultPriority), same parse, same MaxPriorityValue - proto inversion. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
* loadshed: parse LOADSHED_VALVE_ID comment directive in vtgate
MySQL-protocol clients (e.g. Slack webapp) set ExecuteOptions fields via
/*vt+ ... */ comment directives that vtgate parses. The vttablet Snake
load shedder already reads ExecuteOptions.loadshed_valve_id, but nothing
wrote it: the proto field was only reachable by gRPC clients, which get
it via transparent passthrough. Comment-directive clients had no way to
set it.
Teach vtgate to parse a LOADSHED_VALVE_ID directive into the field,
mirroring the existing PRIORITY plumbing: a directive constant, a
QueryHints field, a getLoadshedValveId extractor (free-form string, like
WORKLOAD_NAME), and a SetLoadshedValveId vcursor setter wired through
applyQueryHints. The setter clears the session value when the directive
is absent so a pooled connection can't leak one query's valve ID into
the next.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* loadshed: address review feedback on valve-ID directive
- Simplify SetLoadshedValveId to unconditionally write the parsed valve
ID onto the session options. Overwriting with "" when the directive is
absent still prevents a pooled connection from inheriting a prior
query's valve ID, without the extra clear-on-empty branch.
- Drop the redundant getLoadshedValveId doc comment (restated the name).
- Reword the reused-session test comment to match.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Signed-off-by: Brett Wines <bwines@slack-corp.com>
* loadshed: only touch session options when valve ID is present
The unconditional SetLoadshedValveId (GetOrCreateOptions on every call)
created an empty options:{} on sessions that previously had none, which
broke ~200 golden-comparison tests in go/vt/vtgate (e.g. TestExecutorSet
saw "autocommit:true options:{}" instead of "autocommit:true").
Restore the conditional form mirroring SetPriority: set only when the
valve ID is non-empty, and clear an existing value otherwise, so a
directive-less query never allocates options and a pooled connection
still can't inherit a prior query's valve ID.
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)
Signed-off-by: Brett Wines <bwines@slack-corp.com>
---------
Signed-off-by: Brett Wines <bwines@slack-corp.com>
Snake's self-contention valve stacks a caller's concurrent same-shard queries behind one droppable CoDel representative, in q.valves[valveID]. Those overflow requests are not in the CoDel queue, so the existing length histograms (queue, droppable, holder) are structurally blind to them — there was no metric of any kind for valve state. Add one histogram, per-valve depth: at each valve-keyed enqueue, observe len(q.valves[valveID]) — 0 when the request becomes the representative, N when it stacks N-deep. Percentiles then distinguish one pathological deep valve from many shallow ones. Empty valve IDs bypass the valve and are excluded so structural zeros don't dilute the distribution. Observed at the Acquire enqueue boundary only (rising edge), matching the boundary-read pattern of the other length histograms. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
Health-check/monitoring queries (e.g. against performance_schema) are low-volume and must succeed, but manually tagging every one with a priority is error-prone. Add a schema-based allowlist: a query whose tables reference a configured schema is marked undroppable by the load shedder, so it is never shed regardless of overload. Detection is done once at plan-build time and cached on the plan (Plan.SchemaQualifiers), so the per-request pre-shed path only reads a slice; the common case of unqualified tables is a single length check. The allowlist itself (LoadshedUndroppableSchemas, defaulting to the system schemas) is evaluated live at the Acquire call site, so it stays tunable via /debug/env -- caching a final bool on the plan would go stale when the allowlist changes. Reuses the existing undroppable mechanism by exporting the PriorityUndroppable sentinel. Wires the new --loadshed-undroppable-schemas flag through config, /debug/env, and the vttablet/vtcombo flag docs. Applies to the OLTP-read snake (the path that carries a schema-qualified plan). Adds unit tests for the qualifier extraction and the allowlist match. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Collapse lockedDequeue into lockedRunTimer: the backstop timer and the
release/dequeue path now run the identical CoDel logic, with the paced
drop/ease/re-arm work gated on now>=dropNextNs so an early (not-yet-due)
call is a cheap no-op. The release path calls lockedRunTimer directly.
Rework lockedAdvance as a single per-interval loop that reconstructs the
drop/ease trajectory across a late fire: each interval step drops (while
dropping) or eases (while healthy), then re-evaluates the dropping
presumption from the head's arrival time. The loop runs while a drop is
due AND there is work to do (droppableLen>0 || count>1), so count still
eases to 1 after the queue drains.
Fix the recovery-to-healthy bug: dropping is a per-interval presumption
set when an episode is armed and there is a droppable backlog. Two rules
make it coherent with easing:
- lockedArmDropTimer sets dropping = droppableLen>0 (arming with no
backlog is an ease re-arm, which must not re-assert dropping).
- the end-of-interval head-check only re-arms while count>1; once count
has eased to 1, jump-start mode defers to the monitor as the sole
arming authority (a trigger crossing), matching arm-on-enqueue modes
which re-arm at count==1 as usual.
On a droppable enqueue that takes droppableLen 0->1 mid-easing, catch up
the pending ease steps, then restart the interval from now so the arrival
gets a full interval to prove persistence.
Update the CoDel unit tests to the model: seed a nonzero armed dropNextNs
(0 now means "not armed"), and expect an empty droppable queue to ease
(dropping=false) rather than re-presume dropping each re-arm.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
lockedAdvance incremented count and advanced dropNextNs unconditionally in the drop branch, even when dropFn() failed to shed. When dropFn keeps failing (keep-last refusing the final droppable request, or a droppableLen/index desync) AND the head is old enough that the end-of-interval re-arm check (head.enqueuedAt < dropNextNs) stays true, the loop re-armed dropping every iteration and ran count++ without bound. count ramps until the control-law interval floors at 1ns, at which point the loop advances dropNextNs slower than the real clock moves and never terminates — spinning under s.mu. In production this wedged a tablet (CPU pegged, metrics frozen, ~125k goroutines piled up on s.mu in the Acquire/TxPool.Begin paths behind the release goroutine stuck in lockedCompleteAndShed -> lockedRunTimer -> lockedAdvance). Advance count and dropNextNs only on a successful drop; a refused drop falls through to the easing branch. count can no longer ramp without real drops, so it stays bounded, eases down, and dropNextNs marches past now — the call terminates. This is a dequeue-path bug (not the timer), and is independent of request rate: at steady overload count tops out near the queue depth; the runaway needs a refused drop on a stale head. Adds a regression test that reproduces the spin against a real advancing clock (keep-last + old head + a dropNextNs gap): it spins to count>500M on the old code and terminates with bounded count on the fix. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The CoDel timer/dequeue unification (621dac9) drives shedding off the control-law interval, so with the default 1s test interval the queued valve entries are not shed until roughly an interval elapses. TestPublishStats_ ValveDepthHistogramRecords drains those entries at teardown with a 2s timeout, which the 1s-interval drop no longer beats, so teardown timed out even though the depth-observation assertions (the actual subject) all pass. Give the test a fast CoDel config (1us interval / 1ns target), matching sibling tests like TestPublishStats_DroppableAndIntervalHistogramsRecordUnderLoad, so the queued entries shed promptly at teardown. No assertion changes. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
convertAndLogError built a full SQL + bind-variables log string (queryAsString -> fmt.Sprintf -> BindVariable.String -> prototext.Marshal) on every RESOURCE_EXHAUSTED error, including load-shed rejections. That path is high-volume under overload and its log is rate-limited (logPoolFull, once/minute), so the expensive per-rejection string was built only to be discarded — a CPU profile of an overloaded tablet showed ~18% of CPU in this formatting plus much of the GC it drove. The load shedder sheds to save work under load, then spent more work logging each shed than the shed saved. Skip building the query+bindvars string for RESOURCE_EXHAUSTED: log just the error. The returned client error is unchanged (that branch never included the query), ErrorCounters still increments, and the throttled log still fires — only the discarded per-shed SQL detail is dropped. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
A CPU profile of an overloaded tablet (after the query-stringification fix) still showed ~7% of CPU in vterrors.Errorf -> vterrors.callers -> runtime.Callers, plus the stack-unwind frames it drives, on the load-shed path. Each shed created a vterrors error at the shed site AND had it re-wrapped in convertAndLogError — two runtime.Callers stack captures per rejection, for an expected high-volume outcome. - Pre-build the shed errors as package vars (errLoadShed, errDMLLoadShed), matching the existing errTxThrottled pattern: the stack is captured once at init instead of per rejection, and there is no per-shed allocation. The wrapped snake error was the constant *loadshed.DroppedRequestError, so no per-request detail is lost. - Skip the convertAndLogError re-wrap for RESOURCE_EXHAUSTED with no callerID: return the (pre-built) error as-is instead of a second vterrors.Errorf, avoiding the second stack capture. RESOURCE_EXHAUSTED code, ErrorCounters, and the throttled log are preserved. With the prior stringification fix this takes the shed path from a CPU sink under overload to negligible. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
An execution trace of the loaded primary (with per-CPU intake both on and off) showed ~89% of all scheduler delay in lockedDrop -> Request.signal -> runtime.chansend1: the CoDel drop loop wakes each rejected client goroutine (goready via a channel send) while holding s.mu. With the control-law count ramped high, one drop-timer fire sheds a large batch, so s.mu is held across a long run of goreadys — blocking Acquire (arrivals) and release (grants) for the whole batch. Requests then age in the queue, producing the multi-second shed-latency tail and the semaphore underfill. Split Request.signal into markSignaled (sets signaledValue, under the lock, so every under-lock reader still sees the terminal state synchronously) and sendSignal (the channel send / goready). The batch drop path now marks each dropped request and collects it on q.pendingSignals; the two lockedRunTimer callers (runDropTimer and the release path via lockedFinishRelease) take that slice before unlocking and sendSignal each afterward. Grants and cancels stay single-shot inline (negligible in the trace). This keeps the goready storm out of the critical section so grants and arrivals interleave with reject wakeups instead of serializing behind them. Total wakeups are unchanged; the win is removing the serialization. Per-loop drop cap deferred as a measured follow-up. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…omplete The DML load-shed (Snake) slot acquired in TxPool.Begin was released only via txComplete (the commit/rollback path). Connections torn down through StatefulConnection.Release() directly — the transaction killer, shutdown, taint, and renew-failure paths — bypass txComplete and so leaked their Snake holder. Enough leaks fill the gate's capacity, after which every write waits forever for a grant that never comes: the client times out (~10s) and gets RESOURCE_EXHAUSTED with zero sheds and no lock contention. A goroutine dump from a stalled primary showed exactly this — hundreds of writers parked in Snake.Acquire, the gate otherwise idle, MySQL healthy. Invoke SnakeRelease from ReleaseString, the universal teardown funnel every release path passes through. txComplete still calls it; SafeUnlock.Release is idempotent (once.Do), so the double-call is safe and any conn released without txComplete still frees its slot instead of leaking. Adds a regression test: a conn acquired via Begin and torn down with a direct Release (not Commit/Rollback) must still free the slot — it reproduced the leak before this fix. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
CoDel's drop path always sheds the lowest-priority droppable request, but if the droppable backlog drains to a shallow set it is forced to drop whatever remains — including higher-priority requests. Refuse to shed while the droppable backlog is at or below a fixed floor of 4, so a reserve of droppable requests stays on hand and the lowest-priority ones remain available to shed first. The guard only engages in the queue's near-empty troughs; under a real backlog (droppableLen > 4) shedding is unchanged. The floor changes small-scale shedding behavior (the gate holds up to 4 rather than shedding the last few), so several existing tests that induced shedding at capacity 1 with a handful of contenders are updated: capacity/contender counts raised past the floor so drops still occur and below-floor survivors get granted as holders release, and the grant-stall/valve teardown paths use a cancelable context since floor survivors are never shed while capacity is pinned. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The vttablet_queries latency histogram lumps successful queries together with
fast-failing load-shed rejections (RESOURCE_EXHAUSTED), so there is no exact way
to measure successful-request latency percentiles in isolation — sheds land in
the low buckets and blur the distribution.
Add QueryTimingsByErrorCode, a Timings histogram labeled by the vterrors code of
the result (OK for successful queries, RESOURCE_EXHAUSTED for shed, etc.). It is
recorded in Execute's existing defer from the already-computed errCode and
duration, so it is nearly free. histogram_quantile over {ErrorCode="OK"} now
gives exact granted-only latency, the clean signal for judging whether load-shed
scheduling changes (e.g. yield-on-drop) move the successful-request tail.
Kept as a separate metric rather than folding an error-code label into
vttablet_queries: that is a single-label Timings shared across ~18 call sites
(BEGIN/COMMIT/RESERVE/vreplication phases/...) with no meaningful error code,
and converting it to MultiTimings would pollute those timings and inflate the
hottest histogram's cardinality. Existing dashboards sum by(le) so they are
unaffected either way.
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Add ShedByPriority and AcquireByPriority multi-label counters to Snake, labeled
by the caller's original query priority ("0" most important .. "100" least,
"overflow"). ShedByPriority breaks the scalar ShedCount down so operators can
see whether the gate sheds low-priority traffic first rather than eating
high-priority requests; AcquireByPriority is the offered-load denominator so
per-priority shed rate is ShedByPriority/AcquireByPriority — computed exactly
rather than from assumed traffic weights.
The label reports the caller value: Acquire inverts to the internal Snake
priority (snake = maxPriorityBucket - caller, so lower Snake value sheds first),
and shedPriorityLabel inverts back so the metric matches what the caller passed.
(Hand-ported to this branch, combining the original label commit and the
caller-priority/denominator follow-up; the metric interface gains
NewCountersWithMultiLabels and the test fake implements it.)
Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
histogram_quantile interpolates linearly within a bucket, so a bucket [a,b] bounds the estimate's relative error by b/a. The original shared cutoffs alternated x2 and x5 steps (0.5,1,5,10,50,100,500ms,...), so the x5 buckets — e.g. 100ms->500ms — gave up to 5x error exactly where tablet p99s sit, making the estimate leap by hundreds of ms as samples crossed a boundary (it read as latency noise but was interpolation artifact). Replace with a geometric set: constant multiplier ~10^(1/5) ~= 1.58 (Renard R5 preferred numbers, 1.0/1.6/2.5/4.0/6.3 per decade) across 0.5ms..10s. Every bucket now bounds the quantile to ~1.6x (~±23%) uniformly, so resolution no longer depends on where latency happens to fall. 10 -> 22 cutoffs; a modest cardinality increase over the ~66 shared timing metrics, worth it for p99 you can trust. Regenerated the timings_test expected strings. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Extends the Snake load-test harness so it can exercise regimes the sleep-based model cannot: - -work-mode=cpu: holders busy-spin a calibrated amount of CPU (yielding periodically so they don't monopolize their P) instead of time.Sleep, so they genuinely occupy cores. Under core contention the wall-clock to finish fixed work stretches — the slot-refill delay and latency inflation the sleep model can't produce. -gomaxprocs lets a run saturate a chosen core count. - Per-request priority is threaded into the event log (priority column) so per-priority shed rates can be measured — the axis on which the keep-droppable floor's high-priority protection shows up. All additive: -work-mode defaults to sleep, so existing presets are unchanged. (Hand-ported subset of the rcohen harness commit; the drop_overshoot_ns stats column is omitted because Snake DropOvershootNs is not on this branch.) Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Shedding was driven only by the release path and the backstop timer, so under sparse releases or a late-firing timer the CoDel control law could lag the actual load. Run the control-law advance (lockedRunTimer) on every non-granted enqueue so arrivals drive the drop cadence too. Only the non-granted path advances — an inline grant means there is capacity and nothing to shed. Like the timer path, lockedRunTimer only marks drops, so the pending rejections are drained and sent after s.mu is released to keep the goready storm off the lock. Always on — no config, flag, or /debug/env knob. Also raise MinDropDelay from 1ms to 100ms in both the OLTP-read and DML snakes, flooring how often the drop timer re-arms. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Holder count is bounded by the pool size (testing points to 64 or lower), but it shared the powers-of-two lengthBucketCutoffs (1,2,4,8,16,32,64,...) which collapse that whole operating range into ~6 buckets — no resolution where it matters. Give it a dedicated holderBucketCutoffs with unit steps through the single digits (so underfill to 2 vs 4 vs 6 is distinguishable), tightening steps across the 32-64 range under test, and a little headroom above 64 to catch misconfiguration. Other length histograms (queueLen, droppableLen, dropCount, valveDepth, dropsPerFire) keep the powers-of-two set since they legitimately reach the hundreds. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
…erval-ratio) Replace the absolute --loadshed-interval duration with --loadshed-interval-ratio (float64, default 20): the CoDel observation interval is now derived as target * ratio in both the OLTP-read and DML snakes. This keeps interval tied to target (the recommended 10-20x relationship) instead of requiring the two to be tuned in lockstep. Default 20 * 5ms target = 100ms, matching the prior interval. Updated the tabletenv config field/flag/default, both engine wirings, the /debug/env setter+getVar (now float64), the loadshed-trigger doc that referenced loadshed-interval, the endtoend flag docs, and the tests. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
gofmt-only whitespace fix (align the const block's = signs); no behavior change. Clears a latent gofmt/format-check failure. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
The control-law exponent was tunable via --loadshed-exponent / LoadshedExponent (/debug/env), but 1.0 is the only value we run. Hardcode Exponent to 1 in both the OLTP-read and DML snake configs and remove the flag, config field, default, /debug/env setter+getVar, endtoend docs, and test references. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
… set convertAndLogError re-wraps the returned error with vterrors.Errorf, which captures a stack trace (runtime.Callers) and allocates on every call. The existing RESOURCE_EXHAUSTED fast path that returns the pre-built shed error as-is was gated on callerID == "", but production requests carry an immediate callerID, so shed rejections fell through to the vterrors.Errorf re-wrap and paid a stack capture per drop. A CPU profile of a saturated, heavily-shedding tablet showed vterrors.Errorf -> vterrors.callers at ~5% of CPU, the single largest stack-capture site — precisely when the tablet is already overloaded. Drop the callerID == "" condition: for RESOURCE_EXHAUSTED, always return the pre-built error as-is and fold any callerID into the rate-limited log line only, never into a stack-capturing re-wrap. The returned client error no longer carries the "(CallerID: x)" suffix for shed rejections (an expected, low-value detail the caller already knows); the callerID remains in the server log. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
… floor Both call sites of scheduleDropTimer already floor the arm delay by MinDropDelayNs (100ms in prod) before it reaches lockedScheduleDropTimer, so the additional 5ms backstopFloorNs check could never fire: max(delay, 100ms) is always >= 5ms. The 5ms floor predated MinDropDelay; the 100ms floor added later subsumed it. Remove backstopFloorNs and let MinDropDelayNs be the single, config-visible floor on the drop timer. Co-Authored-By: Claude <svc-devxp-claude@slack-corp.com>
Regenerate query.pb.go / query_vtproto.pb.go (and the vtadmin JS/TS mirrors) for the loadshed_valve_id ExecuteOptions field, and add the --loadshed-* flags to the vttablet/vtcombo help golden files. These generated artifacts were carried as the v25 base version through the rebase; this commit regenerates them against the rebased sources. AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Brett Wines <bwines@slack-corp.com>
bgwines
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background / Why?
Snake is the CoDel-based load-shedding gate that fronts vttablet's OLTP read and transaction connection pools, shedding excess work when the pool is saturated so capacity doesn't collapse under overload. It has run in production at Quip since mid-2022 and is proposed for upstream in the vttablet admission-control RFC.
The gate was developed on
slack-22.0(PR #860). This branch replays that work onto v25mainso it can serve as the base for the framework-integration prototype (#907), which needs both Snake and the v25-onlyquerythrottlerframework present on one branch. Keeping the rebase separate lets that PR's framework delta be reviewed on its own rather than buried under Snake's diff.The rebase itself is almost entirely the new
loadshed/package (which lands conflict-free, as it adds no files that exist onmain); the only reconciliation against three majors of drift was in the ~15 integration files (query_executor.go,tabletserver.go,tx_pool.go,tabletenv/config.go,proto/query.proto, and the stats/timings helpers). Notable v25 adaptations: theconvertAndLogErrorlogging path migrated from printf-style to structuredslog, and the timing-histogram buckets became geometric — both reconciled to keep v25's shape while preserving Snake's behavior.Not intended to merge to
mainas-is; it's the shared base for the prototype.Testing
existing unit tests updated
AI disclosure: Claude Code assisted with development. Every line of code was either written by or carefully reviewed by me :)