Skip to content

issue #547: wire jvector readRangeAsync to RemoteFileServiceClient for parallel FusedPQ IO#549

Merged
eolivelli merged 3 commits into
masterfrom
issue-547-wire-jvector-readrangeasync-to-remotefi
May 13, 2026
Merged

issue #547: wire jvector readRangeAsync to RemoteFileServiceClient for parallel FusedPQ IO#549
eolivelli merged 3 commits into
masterfrom
issue-547-wire-jvector-readrangeasync-to-remotefi

Conversation

@eolivelli
Copy link
Copy Markdown
Owner

Fixes #547.

Changes

  • .github/workflows/ci.yml + kubernetes-tests.yml: pin jvector checkout to parallel-fusedpq-io branch while eolivelli/jvector#8 is unmerged. Both workflows had ref: main; must track the PR branch to pick up the new readRangeAsync API. TODO: revert to ref: main once the PR merges.

  • SegmentBlockCache: add AsyncBlockLoader functional interface + getBlockAsync(path, offset, length, AsyncBlockLoader) method. Uses a ConcurrentHashMap<BlockKey, CompletableFuture<ByteBuf>> inFlightAsync for single-flight deduplication: concurrent misses for the same block share one loader call, each getting an independent retained slice. Hit path is an atomic computeIfPresent retain under the Caffeine map lock — the same pattern as getBlock. Cache insertion uses compute() to handle the rare race with a concurrent sync getBlock call. RefCnt ownership rules (refCnt=2 on insert: cache ref + caller ref) are identical to the sync path.

  • RemoteRandomAccessReader: override readRangeAsync(long, int) from jvector's RandomAccessReader. Does NOT touch position, blockBuffer, or bufferedBlockIndex (async reads bypass the sliding-window cursor). Single-block fast path calls one getBlockAsync; multi-block path fires all covering blocks in parallel with allOf, assembling a heap ByteBuffer in the completion callback. All ByteBuf slices are released in a finally block (both success and failure paths) so no off-heap memory escapes. fetchBlockFromRemoteAsync helper mirrors fetchBlockFromRemote — clamps length to avoid reading past EOF, updates rfs_client_read_* counters and VectorSearchRequestContext accounting.

  • PersistentVectorStore: add static volatile boolean searchAsyncPipelineEnabled with setSearchAsyncPipelineEnabled() / isSearchAsyncPipelineEnabled(). Mirrors the setStreamingCompactionEnabled pattern. Default false until a production profile confirms a net win.

  • VectorSegment.search(): after getting or creating the ThreadLocal-cached GraphSearcher, call searcher.setAsyncPipelineEnabled(PersistentVectorStore.searchAsyncPipelineEnabled) on every search invocation. Cheap field write; enables runtime toggle without discarding cached searcher instances.

  • IndexingServerConfiguration: add PROPERTY_VECTOR_SEARCH_ASYNC_PIPELINE_ENABLED / SYSPROP_VECTOR_SEARCH_ASYNC_PIPELINE_ENABLED constants (default false).

  • IndexingServiceEngine: read the new property at IS startup and call PersistentVectorStore.setSearchAsyncPipelineEnabled(); config key takes precedence over system property. Logs the resolved value.

Tests

  • RemoteRandomAccessReaderAsyncTest (11 cases, Netty PARANOID leak detector): readRangeAsync returns identical bytes to seek+readFully for single-block, cross-block, three-block, and end-of-file ranges; position is invariant after async reads; 16 concurrent async reads alongside a synchronous readFully loop on the same reader instance produce correct bytes with no refcount exceptions; cache warm-up via sync path is visible to subsequent async reads; disabled-cache pass-through works.

  • SegmentBlockCacheAsyncTest (7 cases, Netty PARANOID leak detector): pass-through on disabled cache; miss populates then next call is a hit; sync miss populates then async call is a hit; 16 concurrent async misses share exactly one loader call; each concurrent caller receives an independent ByteBuf slice; loader failure propagates to all waiting callers; multi-threaded async loads are byte-correct.

🤖 Implemented by the pr-worker agent.

eolivelli added 3 commits May 13, 2026 10:49
…r parallel FusedPQ IO

- Pin CI jvector checkout to parallel-fusedpq-io branch while eolivelli/jvector#8 is unmerged
- SegmentBlockCache: add AsyncBlockLoader FI + getBlockAsync() with single-flight semantics
  via ConcurrentHashMap<BlockKey, CF<ByteBuf>> inFlightAsync; hit/miss/load-time counters
  mirror the sync getBlock() path; correct refCnt=2 ownership on cache insertion
- RemoteRandomAccessReader: override readRangeAsync(long, int) — does NOT touch position/
  blockBuffer/bufferedBlockIndex; single-block fast path + multi-block allOf path with
  ByteBuf release-in-finally; fetchBlockFromRemoteAsync helper updates rfs_client_read_*
  stats; VectorSearchRequestContext hit/miss/readFileRange accounting preserved
- PersistentVectorStore: add static volatile searchAsyncPipelineEnabled flag with
  setSearchAsyncPipelineEnabled() / isSearchAsyncPipelineEnabled() (default false)
- VectorSegment.search(): call searcher.setAsyncPipelineEnabled() on every search so
  runtime toggle takes effect without discarding ThreadLocal-cached searchers
- IndexingServerConfiguration: add PROPERTY_VECTOR_SEARCH_ASYNC_PIPELINE_ENABLED /
  SYSPROP_VECTOR_SEARCH_ASYNC_PIPELINE_ENABLED constants (default false)
- IndexingServiceEngine: read new property and call setSearchAsyncPipelineEnabled() at IS startup
- New tests: RemoteRandomAccessReaderAsyncTest (11 cases, PARANOID leak detector) +
  SegmentBlockCacheAsyncTest (7 cases, PARANOID leak detector)
Correctness fixes:
- RemoteRandomAccessReader.fetchBlockFromRemoteAsync: convert null result
  (block-not-found) to a failed future with IOException, mirroring the
  null-check in the synchronous fetchBlockFromRemote; register the failure
  on clientReadLatency so the Grafana panels stay consistent
- SegmentBlockCache.getBlockAsync: wrap loader.loadAsync() invocation in
  a try/catch (narrow RuntimeException) so a synchronously-throwing loader
  cannot leak the inFlightAsync entry (deadlocking future piggybackers).
  On synchronous throw: increment loadFailure, remove inFlightAsync slot,
  complete ourFuture exceptionally, return
- RemoteRandomAccessReader.readRangeAsync multi-block path: capture
  wasCached + blockLen per block before dispatch, then record per-block
  cache-hit / cache-miss / readFileRange events on VectorSearchRequestContext
  after allOf completes — matches the single-block path contract documented
  in the readRangeAsync docstring
- RemoteRandomAccessReader.readRangeAsync multi-block path: narrow
  catch (Exception e) to catch (RuntimeException e) with a justifying
  comment, per CLAUDE.md narrow-catch rule
- Replace inline lambdas with this::fetchBlockFromRemoteAsync method
  references (readability)

New tests (PARANOID Netty leak detector):
- RemoteRandomAccessReaderAsyncTest#testMultiBlockPartialFailureReleasesAllBuffers:
  block 0 cache-hit + block 1 network-miss (server stopped, retries=0) —
  asserts future fails and no ByteBuf leaks
- RemoteRandomAccessReaderAsyncTest#testMultiBlockUpdatesVectorSearchRequestContextStats:
  cross-block read with block 0 warmed asserts 1 hit + 1 miss + 2
  readFileRange events recorded on VectorSearchRequestContext
- SegmentBlockCacheAsyncTest#syncThrowingLoaderDoesNotDeadlockNextCaller:
  synchronously-throwing loader → future fails AND inFlightAsync slot is
  cleared so a subsequent getBlockAsync with a working loader invokes it
- SegmentBlockCacheAsyncTest#concurrentSyncAndAsyncMissInsertSeesSameBytes:
  race a sync getBlock with an in-flight async load on the same key,
  assert byte equality and zero leaks (exercises the compute()-race path)

Test hygiene:
- SegmentBlockCacheAsyncTest: cache.clear() + cleanUp() in @after so
  PARANOID leak detection deterministically observes any leaked ByteBufs
- RemoteRandomAccessReaderAsyncTest: handle null server in @after (for
  tests that stop the server mid-test); fix assertFalse → fail() for
  clarity in testReadRangeAsyncPastEofFails
The parallel-fusedpq-io PR branch was merged into main, so both CI
workflows can go back to checking out main. Drops the transient
ref: parallel-fusedpq-io pin (and the explanatory comment) introduced
earlier in this PR.
@eolivelli eolivelli merged commit 9abe408 into master May 13, 2026
13 of 15 checks passed
@eolivelli eolivelli deleted the issue-547-wire-jvector-readrangeasync-to-remotefi branch May 13, 2026 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Wire jvector readRangeAsync to RemoteFileServiceClient for parallel FusedPQ search IO

1 participant