Repository navigation
Conversation
fcostaoliveira
added a commit
that referenced
this pull request
Jul 19, 2026
…readiness (supersedes #145/#146/#147) (#148) * Add fixed-rate open-loop search mode * Improve Vertex transport and duration benchmarks * fix(bench): fair Redis-vs-Vertex measurement + single-shard/high-QPF readiness Applies the fixes from an adversarial review of the stacked Vertex PRs (#145/#146/#147). Every measurement asymmetry in the open-loop / duration search harness ran one direction — it understated Redis — and there was no clean operating point to drive a fast single-shard/high-QPF Redis. Changes: Fairness (blocker/major): - Redis prime + two-barrier clock start (open-loop AND closed-loop-duration), mirroring Vertex: every worker connects + issues a discarded warm-up FT.SEARCH, then the measurement clock is stamped and released only once all workers are warm. Connection setup and the cold first query now fall OUTSIDE total_time and the latency percentiles. Barrier is deadlock-safe: a worker that fails to connect still waits both barriers. - Hoist all Redis client-side request construction out of the timed window: configured_index_name() resolved once (was a per-query env::var), and FT.SEARCH assembly split into build_ft_search_cmd() (pre-timer) + exec, so the timed window wraps only the RPC round-trip + reply parse, matching the Vertex boundary. - client_saturated is now decided ONLY by measured signals (client/system CPU); the pure parallel>cores "oversubscribed" term is demoted to an advisory field so a legitimately high-concurrency run against a fast shard is not auto-flagged. Correctness / robustness: - num_candidates clamps to >= top (was max(1)); Vertex requires approximateNeighborCount >= neighborCount. - Redis recycled-index length guard (clean Err instead of a worker panic). - Redis search connections get 120s read/write socket timeouts. - Overload-shed runs report rps=0 with drop/schedule-delay accounting instead of Err("No searches completed"). - Dropped requests no longer double-count as late / pollute schedule_delay. - --warmup-seconds allowed in closed-loop-duration mode (discarded warm-up phase for both engines); open-loop rep selection no longer keys on rps. - Reject non-finite/absurd --search-duration/--max-lateness-ms/--warmup-seconds. - CPU-sample window caveat documented (brackets whole search()). Verified: cargo build clean, cargo clippy 0 warnings, cargo fmt applied, 477 unit tests pass. (integration_dragonfly needs a live Docker service.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Lionel Giavelli <3373753+kaizer113@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 19, 2026
Collaborator
|
Superseded by #148 (merged as 8303f1d), which consolidates this PR's code and applies the fairness/correctness fixes from a 24-agent adversarial review of the #145/#146/#147 stack (Redis prime+barrier symmetry, timed-window hoist, client_saturated fix, num_candidates clamp, overload-shed reporting, etc.). Closing as its functionality is now on master. |
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
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.
Summary
Why
The existing REST-only client included public-network and HTTP overhead in Vertex measurements and the harness could only run a fixed query count or fixed-rate open-loop workload. These changes let benchmarks use Vertex's native private gRPC path and measure maximum sustainable throughput for a fixed duration while preserving preloaded indexes between runs.
Impact
Set
VERTEX_QUERY_TRANSPORT=private-grpcwithVERTEX_GRPC_ADDRESSto benchmark a PSC/VPC endpoint, or usepublic-grpcfor a public endpoint. Passing--search-durationwithout--target-qpsnow cycles queries in unrestricted closed loop for Redis and Vertex;--keep-dataprevents cleanup andREDIS_INDEX_NAMEselects an existing Redis index.Vertex uploads now retry HTTP 429 quota responses, and timed workers establish their connections before the shared measurement clock begins.
Dependency
This is a follow-up to #146 and currently includes that PR's commit. Once #146 merges, GitHub will reduce this PR to the single follow-up commit automatically.
Validation
make checkmake vector-db-benchmarkmake test— 79 passedmake integration-test— 24 passed