Snake: CoDel-based load shedding gate for vttablet conn pools - #860
Draft
bgwines wants to merge 76 commits into
Draft
Snake: CoDel-based load shedding gate for vttablet conn pools#860bgwines wants to merge 76 commits into
bgwines wants to merge 76 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## slack-22.0 #860 +/- ##
==============================================
+ Coverage 69.52% 69.57% +0.05%
==============================================
Files 1606 1610 +4
Lines 214357 214752 +395
==============================================
+ Hits 149021 149413 +392
- Misses 65336 65339 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bgwines
force-pushed
the
bwines/snake-22
branch
2 times, most recently
from
May 21, 2026 21:14
0de8af7 to
8ac9b6d
Compare
### Background / Why? 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. ## Architecture ``` 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 ``` ## Request lifecycle events | 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 | ## Differences from the Python implementation 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 ## Other changes - **`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. ## Testing ~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>
bgwines
force-pushed
the
bwines/snake-22
branch
2 times, most recently
from
June 18, 2026 01:07
fbb9a86 to
95a1721
Compare
bgwines
changed the title
EasingDivisorExpose 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>
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>
* loadshed: judge CoDel re-arm staleness by oldest waiter, not raw list head lockedAdvance's head-check compared the raw list front's enqueue time against the next drop deadline to decide whether to keep dropping. A granted request stays resident in the list as UNDROPPABLE until Release, so a long-held straggler could be the raw list front — its age drove the staleness decision even though it can never itself be a drop candidate. During a multi-interval catch-up (e.g. a backstop timer that fired late), this let a single stuck holder ramp count and shed an otherwise-healthy backlog. Judge staleness by the oldest WAITING request instead. 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> * remove slop comments Signed-off-by: Brett Wines <bwines@slack-corp.com> * loadshed: trim remaining verbose assert messages Signed-off-by: Brett Wines <bwines@slack-corp.com> --------- Signed-off-by: Brett Wines <bwines@slack-corp.com>
### Background / Why? CoDel measures sojourn when a request is granted, so an executing request contributes no new queue-health information after grant. This PR is a refactor; it has no behavioral change. It removes granted requests from the CoDel list at grant time while leaving Snake's semaphore and holder lifecycle unchanged; release still performs the holder and valve accounting that belongs to the current Snake API. Admission decisions therefore remain unchanged. This is a step in preparation for replacing `wl.list` from `go/pools/smartconnpool/waitlist.go` with the snake. In that world, the semaphore is the conn pool itself. ### Testing Existing unit tests updated. AI assisted with development. Every line of code was either written by or carefully reviewed by me :)
Make Snake configurable per pool
Keep Snake load shedding slow-start only
AI 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>
AI 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>
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
3 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.
vitessio#20639