perf(test): cut round-trips in HNSW vector-index integration tests#1505
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request optimizes the vector-index-integrity integration tests by parallelizing independent read-only vector searches using Promise.all and batching record insertions into a single bulk request instead of executing them sequentially. These changes significantly improve test execution performance. There are no review comments, so I have no additional feedback to provide.
Contributor
|
Reviewed; no blockers found. |
The vector-index-integrity suite is a major Windows CI hotspot (~29 min for the suite). Most of the cost is ~100–200 sequential, individually-committed HTTP round-trips per test — tiny data (DIMS=8, N≤50), so it's round-trip + per-write fsync overhead, not vector math. Apply correctness-neutral reductions: - reindex backfill: bulk-insert the N pre-index seed records in one request instead of N sequential POSTs. There is no HNSW index during insert, so per-record ordering is irrelevant; the backfill builds the index afterward. - update-churn & reindex: run the independent read-back verification searches concurrently (Promise.all) instead of serializing N round-trips. Left untouched (semantically order-sensitive): the entry-point insert/delete sequence and the churn update sequence. Note: validated on CI, not locally — restart_service is flaky in a local multi-loopback setup (readiness probes time out at 60s), which both fails and dominates local timings; the churn test (the one that runs cleanly locally) still passes with the parallelized read-back. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dawsontoth
force-pushed
the
claude/perf-vector-tests
branch
from
826779e to
9bf4a5b
Compare
Each test in this suite deployed its own component and restarted the HTTP workers; the restart is the dominant per-test cost on slow runners (a test doing 3 inserts still took ~265s on Windows — almost entirely restart/setup). Deploy all four tables in one component with a single restart in `before`, and drop the per-test add_component/set_component_file/restart. Restarts: ~5 -> 2 (the shared one, plus the single restart `reindex` needs for its mid-test index-add); add_component: 4 -> 1. Tables keep distinct names/databases so the tests stay isolated despite sharing one component. ReindexTable is deployed without an index initially; the reindex test redeploys the full schema with it indexed (a no-op re-assert for the other three, which have already run — reindex is last) to trigger backfill. CI-validated (local restart_service is flaky in a multi-loopback setup). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
github-actions
Bot
requested review from
Ethan-Arrowood,
cb1kenobi,
heskew and
kriszyp
This was referenced Jun 26, 2026
kriszyp
approved these changes
Jun 28, 2026
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.
Measured result (CI, Windows shard 5/6)
The restart-reduction restructure (deploy all tables once, ~5 worker restarts → 2) delivered the win:
All four vector tests pass on v24/v26/Windows. Per-test cost was almost entirely the per-test worker restart; the residual ~422s is the single shared
beforedeploy+restart. (The round-trip reductions — bulk insert + parallel read-back — are a secondary, in-the-noise contribution on top.)Status: Draft — Windows CI perf for the vector-index integration tests. Stacked on #1504 (set as this PR base), so its shard-5/6 hardening is in scope and the perf change is validated without the pre-existing flake noise. Independent of the RFC PR #1503.
Why
integrationTests/server/vector-index-integrity.test.tsis a top Windows CI hotspot — theHNSW vector-index data-integritysuite runs ~29 min on Windows 5/6 (1763s), with individual tests at 284s/237s/184s.The data is tiny (DIMS=8, N≤50). The cost is ~100–200 sequential, individually-committed HTTP round-trips per test — i.e. per-request round-trip + per-write fsync, not vector math. Measured ratio: ~6× slower on Windows than local for the same suite.
Changes (correctness-neutral)
insertop. There's no HNSW index during these inserts (the table is altered to add the index afterward), so per-record ordering is irrelevant; the backfill builds the index from the stored rows either way.update-churnandreindexverification loops issue N independent QUERY searches; run them withPromise.allinstead of serializing the round-trips.Deliberately left sequential (order-sensitive, would change what's tested): the delete-entry-point insert/delete sequence (entry-point election) and the churn update sequence.
Validation
restart_serviceis flaky in a local multi-loopback setup — the per-test readiness probes time out at 60s, which both fails and dominates local timings (an artifact that doesn't exist on CI, where these tests pass). Theupdate-churntest — the one that runs cleanly locally — still passes with the parallelized read-back. The real before/after is the Windows 5/6 shard time on this PR's CI run vs the ~1763s baseline.Bigger levers (not in this PR)
restartHttpWorkers— each test restarts workers (~5 across the suite; reindex does 2). Worker-restart cost is likely a larger driver than the data ops, but reducing it means sharing setup across tests (changes test isolation) — worth a follow-up.🤖 Generated with Claude Code