issue #599: bypass block-cache for PQ retraining via local segment file download#601
Merged
eolivelli merged 2 commits intoMay 19, 2026
Merged
Conversation
…nt files locally During streaming compaction, PQRetrainer.extractVectorsSequential() called getVectorInto() once per sampled node, each of which issued a gRPC block-cache read over remote storage. For 13 source segments × ~4 096 samples each, this serialized ~53 248 network round-trips. Option B: download each source graph file once to a temp file in the IS data directory (store.tmpDirectory()), then open all PQ-retraining Views from that local file. When the DSM supports direct object-storage download (supportsDirectMultipartDownload=true), the temp file is written without going through the gRPC file server at all; otherwise, a single sequential DSM read buffers the entire file for the retrainer. New classes: - DeleteOnCloseReaderSupplier: wraps a ReaderSupplier and deletes the backing temp file on close() - SegmentPQReaderSupplier: builds the Function<OnDiskGraphIndex,ReaderSupplier> factory used by OnDiskGraphIndexCompactor (via the new 6-arg constructor introduced in eolivelli/jvector #14) Changes to existing classes: - VectorIndexCompactor: wire SegmentPQReaderSupplier.forSegments() into rebuildSegmentStreaming(); add PQ_BULK_READER_COUNT counter for test observability - PersistentVectorStore: expose segmentStorageKey(), tableSpaceUUID(), and dataStorageManager() as package-private accessors for SegmentPQReaderSupplier Tests: - VectorIndexStreamingCompactionTest#streamingCompactionUsesBulkPQReaderSupplier: exercises the full runCompactionCycle() → rebuildSegmentStreaming() → PQRetrainer path with MemoryDataStorageManager and verifies that the bulk reader factory is invoked at least twice (once per source segment) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SegmentPQReaderSupplier: add candidates/sources size mismatch precondition check (IllegalArgumentException) and graphFileSize <= 0 guard (IllegalStateException); remove redundant deleteSilently from catch blocks (only in finally); replace bare RuntimeException with IllegalStateException - VectorIndexCompactor: move PQ_BULK_READER_COUNT increment to after pqReaderFactory.apply() returns successfully; use short AtomicInteger name - SegmentPQReaderSupplierTest: new test class covering sizeMismatchRejected, zeroGraphFileSizeRejected, directDownloadFastPathSuccessCleansTempFile, and directDownloadFastPathFailureCleansTempFile — the last two use a DirectDownloadDsm that returns supportsDirectMultipartDownload()=true and only injects failures for fileType="graph" (PQ path), not "map" (checkpoint) - VectorIndexStreamingCompactionTest: update Javadoc for streamingCompactionUsesBulkPQReaderSupplier to reflect that the counter reflects successful reader acquisitions, not just factory invocations Co-Authored-By: Claude Sonnet 4.6 <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.
Fixes #599.
Changes
DeleteOnCloseReaderSupplier(new): wraps aReaderSupplierand deletes the backing temp file onclose(); ensures temp segment files downloaded for PQ retraining are cleaned up after useSegmentPQReaderSupplier(new): builds theFunction<OnDiskGraphIndex,ReaderSupplier>factory injected intoOnDiskGraphIndexCompactor; for each source segment, either downloads the graph file directly from object storage (whenDataStorageManager.supportsDirectMultipartDownload()istrue) or opens a sequential reader viaDataStorageManager.multipartIndexReaderSupplier(); temp files live instore.tmpDirectory()(= IS data directory)VectorIndexCompactor: wireSegmentPQReaderSupplier.forSegments()intorebuildSegmentStreaming()via the new 6-argOnDiskGraphIndexCompactorconstructor (introduced in issue #599: add readerSupplierFactory to PQRetrainer / OnDiskGraphIndexCompactor jvector#14); addPQ_BULK_READER_COUNTAtomicIntegercounter for test observabilityPersistentVectorStore: exposesegmentStorageKey(),tableSpaceUUID(), anddataStorageManager()as package-private accessors needed bySegmentPQReaderSupplierTests
VectorIndexStreamingCompactionTest#streamingCompactionUsesBulkPQReaderSupplier: creates a store withMemoryDataStorageManager(dim=16, 300 vectors × 5 checkpoints so FusedPQ is enabled), callsstore.runCompactionCycle(), and asserts thatPQ_BULK_READER_COUNTincreased by ≥ 2 (one per source segment) and that a search on the resulting index returns non-empty resultsDepends on
eolivelli/jvector#14 — the 6-arg
OnDiskGraphIndexCompactorconstructor andOnDiskGraphIndex.getView(ReaderSupplier)overload🤖 Implemented by the
pr-workeragent.