Reduce disk reads on the tiered Vector Set search path - #2007
Open
badrishc wants to merge 5 commits into
Open
Conversation
On the storage-tiered (disk-backed) DiskANN search path, per-rerank-candidate disk reads dominated query cost even though the quantized vectors and graph adjacency were already memory-resident: - The DiskANN Metadata term is a small, shared record read once per rerank candidate, but it fell into the default read-copy branch (CopyTo=None) and was re-read from disk on every query. Add it to the copy-to-tail branch alongside the other per-node graph records (NeighborList, QuantizedVector, id maps) so it is served from memory after first touch, bounding disk traffic to the visited working set. Making DiskANNService.Metadata internal so the policy can name it. - The FullVector's initial disk read could require a second IO when the record overhead exceeded the 64-byte budget. Widen VectorRecordReadOverheadBytes to 128 so the whole record (RecordInfo + RecordDataHeader + namespace + key + value) lands in a single sector-aligned IO. Over-sizing is free here (it rounds to the same sectors as the value alone); under-sizing forces a second IO. Measured on Cohere-10M (768-dim, Q8, l_search=192), warm disk-tiered serial: disk bytes/query 2,176 -> 594 KB (-73%), warm serial latency 86 -> 46.5 ms, disk peak QPS ~223 -> ~1,162, recall unchanged (0.8408). Vector Set recall smoke tests pass across NOQUANT/Q8/BIN on the evict / copy-to-tail / recover paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81de0152-2aa0-47b5-bf27-f0358619d578
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the disk-backed DiskANN Vector Set read path by reducing repeated storage-tier disk reads and improving initial disk read sizing for full-vector records.
Changes:
- Cache DiskANN “Metadata” term records via read-copy (copy-to-tail/read-cache) so they are served from memory after first touch.
- Increase
VectorRecordReadOverheadBytes(64 → 128) to better ensure a single sector-aligned IO fetches the whole FullVector record. - Make
DiskANNService.Metadatainternalso the read-copy policy can reference it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| libs/server/Resp/Vector/VectorManager.Callbacks.cs | Adds Metadata to the read-copy policy and widens the initial disk-read sizing overhead used for vector-record reads. |
| libs/server/Resp/Vector/DiskANNService.cs | Exposes the Metadata term constant at internal visibility for use by the vector read-copy policy. |
…lignment slack Address PR review feedback: the initial-read-size overhead is not unconditionally free. Downstream Tsavorite rounds the requested length up to the device sector size, so the extra bytes add no IO only while value+overhead rounds up to the same sectors the full record occupies; a bump that crossed a sector boundary would read an additional sector. Doc-comment only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81de0152-2aa0-47b5-bf27-f0358619d578
The initial-read-size overhead was bumped 64 -> 128 as a precaution, but it was not justified: 64 already fetches the FullVector in a single IO. The on-disk record is RoundUp(16 header + <=4 namespace + 4 key + 3076 value, 8) = 3104 B, and the initial request at overhead 64 is dims*4 + 64 = 3136 >= 3104, so the whole record lands in one read. Confirmed via iostat on Cohere-10M/Q8: ~212 device reads/query at ~3.37 KB each (== the ~193 logical FullVector reads, not 2x), recall unchanged. Because the read is rounded up to the device sector size, the 128 setting only risked pulling an extra 512 B sector for the ~12% of records whose end falls in the extra 64 B window -- a small cost for no benefit. Revert to 64 (matching main) and document the exact per-record budget. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81de0152-2aa0-47b5-bf27-f0358619d578
Drop history/measurement asides from the read-copy and read-overhead comments and correct the read-copy policy: the Metadata term is a small shared structure, not a per-node block. Comment-only; no behavior change. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 81de0152-2aa0-47b5-bf27-f0358619d578
kevin-montrose
approved these changes
Aug 3, 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
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.
What
Cache the DiskANN Metadata term on the storage-tiered (disk-backed) Vector Set read path. It previously fell into the default read-copy branch (
CopyTo=None) and was re-read from disk on every rerank candidate; it is now copied back to memory after first touch like the other graph records reused during search (NeighborList, QuantizedVector, internal/external id maps).DiskANNService.Metadatais madeinternalso the read-copy policy can reference it.Why
On the disk tier, even though the quantized vectors and graph adjacency are already memory-resident, two records were read from disk once per rerank candidate: the raw FullVector (expected — used for the exact-distance rerank) and the DiskANN Metadata term. The Metadata term is a small, shared structure (~154 records for a 10M graph) but at ~8 KB each and read ~193×/query it accounted for ~73% of disk bytes/query — more bytes than the vectors themselves — purely because it was never cached. Caching it costs ~1.3 MB of memory and removes it from the steady-state disk path entirely.
Impact
Cohere-10M (768-dim, COSINE), Q8,
M=16 / l_build=300 / l_search=192 / k=100, warm disk-tiered serial (--storage-tier, Native/O_DIRECT):Tests
Garnet.test.vectorset— theVectorSetRecallSmokeTests(recall survives eviction / copy-to-tail / save+recover across NOQUANT/Q8/BIN) pass on both net8.0 and net10.0; the full vectorset suite is green.Notes / follow-up (not in this PR)
After this change the sole remaining steady-state disk cost is the FullVector rerank reads, which DiskANN currently issues one key at a time (the read callback is invoked with
numKeys=1, ~193 serial calls/query), capping the device queue depth well below the drive's IOPS ceiling. Garnet's batched-read path (ReadWithPrefetch) already parallelizes reads fornumKeys>1(used for graph traversal today), so batching the rerank GET would be a small, self-contained DiskANN-side change with no Garnet change required — a promising follow-up for a further QPS lift.