chatstress: fix query rate control + add fuzzer/oracle correctness tests#4
Open
kei-nan wants to merge 3 commits into
Open
chatstress: fix query rate control + add fuzzer/oracle correctness tests#4kei-nan wants to merge 3 commits into
kei-nan wants to merge 3 commits into
Conversation
Query workers previously self-paced with a per-worker pre-sleep of (rate/threads) seconds between queries. That sleep runs in series with each query's own latency, so at high per-query latency the delivered rate fell well short of the target (e.g. ~570 q/s for a 1500 q/s setting). Replace it with one shared live-rate token bucket: tokens are produced on a fixed tick independent of query latency, so N workers blocking on it deliver the configured aggregate rate, bounded only by what the workers can sustain. Rate 0 stays unlimited, and the producer keeps the bucket topped up in that mode so workers already blocked from a previous non-zero rate are released promptly (otherwise switching to unlimited stalled them). Add two correctness tests: - fuzz: property test over 20k generated queries asserting none ever emit a disk-illegal argument (SUMMARIZE/HIGHLIGHT/SLOP/INORDER/GEOFILTER/ WITHCURSOR/WITHSUFFIXTRIE/PAYLOAD; FT.SEARCH SORTBY or numeric FILTER; wildcard/prefix/lexrange TAG content) and that both FT.SEARCH and FT.AGGREGATE are exercised. - oracle: table test for the expiry (t0 + clock-skew) and deletion (grace window) classification rules plus the stale-hit counters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rate limiter: the earlier "keep the bucket topped up while unlimited" change regressed rate control under real (non-trivial) query latency. Topping the 4096-slot bucket up, plus surplus tokens accumulating whenever workers are latency-bound and can't keep up, left a large backlog of stale permits; after lowering the rate (or leaving unlimited) workers burned through that backlog at full capacity for many seconds before the new rate took effect. Local in-RAM tests missed it because sub-millisecond queries drain the bucket instantly. Fix: flush buffered permits on any rate drop (including ->0), and make Wait() poll the live rate while blocked so switching TO unlimited never strands a worker on the token channel — removing the top-up hack entirely. Verified against a high-latency endpoint: rate=25 -> ~25 q/s worker rate (was ~3x over), rate=0 -> full capacity with no stall. Connection: add a full-URL input (redis://|rediss://) parsed via redis.ParseURL, so a cloud endpoint can be supplied as one string. Precedence: -url flag > $CHATSTRESS_URL > config `url` > `addr`. The URL/password is never logged (only host:port). Add .env (gitignored) + .env.example so the secret stays out of shell history and git; document it in the README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cmd/chatstress/main.go and reload.go (the program entrypoint + reload-check mode) were never committed: the .gitignore pattern `chatstress` — meant for the built binary — also matched the cmd/chatstress/ source directory and silently excluded them, so a fresh clone had no `main` package and could not build. Anchor the pattern to `/chatstress` and add the two files. Completes the URL support from the previous commit (the -url/$CHATSTRESS_URL wiring lives in main.go). Verified: a clean export of the committed tree builds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.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
1 participant
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.
Follow-up to #2 (a second review pass on the use-case-6 chat/messaging stress harness).
Query rate control (bug fix)
Query workers self-paced with a per-worker pre-sleep of
(rate / threads)seconds between queries. That sleep runs in series with each query's own latency, soeffective ≈ rate / (1 + rate·latency/threads)— the delivered rate fell well short of the target whenever per-query latency was non-trivial (the disk path, precisely the case we're stressing).Replaced with a single shared live-rate token bucket: tokens are produced on a fixed tick, independent of query latency, so N workers blocking on it deliver the configured aggregate rate — bounded only by what the workers can actually sustain. Rate
0stays unlimited, and the producer keeps the bucket topped up in that mode so workers already blocked from a previous non-zero rate are released promptly (an early cut of this fix stalled at0; now fixed and verified).Verified against an in-RAM smoke target:
Tests
SUMMARIZE/HIGHLIGHT/SLOP/INORDER/GEOFILTER/WITHCURSOR/WITHSUFFIXTRIE/PAYLOAD;FT.SEARCHSORTBYor numericFILTER; wildcard/prefix/lexrange TAG content), and bothFT.SEARCHandFT.AGGREGATEare exercised.t0+ clock-skew) and deletion (grace-window) classification rules, plus the stale-hit counters.gofmt/go vet/go test ./.../go buildall clean.🤖 Generated with Claude Code