Skip to content

fix: dispatch concurrent blob range reads#7861

Merged
Xuanwo merged 2 commits into
mainfrom
xuanwo/fix-blob-range-concurrency
Jul 20, 2026
Merged

fix: dispatch concurrent blob range reads#7861
Xuanwo merged 2 commits into
mainfrom
xuanwo/fix-blob-range-concurrency

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

Concurrent BlobFile range reads against the same physical source were serialized between opportunistic batches because BlobSource waited for each batch to finish I/O before draining requests that arrived while it was in flight. This made caller concurrency scale poorly even though the Python binding releases the GIL and the existing scheduler can execute the reads concurrently.

Dispatch each drained batch without waiting for earlier batches to finish, while leaving FileScheduler responsible for I/O concurrency and byte backpressure. Deterministic same-source and multi-source gated tests cover the batching boundary and existing order and cursor behavior.

Real S3 impact

Measured on Amazon S3 in us-east-2 with 100 KiB range reads at concurrency 64:

Configuration Before After Throughput gain p50 before p50 after p50 reduction
Dedicated / same source 876 reads/s 2,531 reads/s 2.89x 64.7 ms 23.9 ms 63.1%
Dedicated / multiple sources 1,169 reads/s 2,505 reads/s 2.14x 53.5 ms 24.0 ms 55.1%
Packed / same source 1,004 reads/s 2,463 reads/s 2.45x 59.4 ms 24.6 ms 58.5%
Packed / multiple sources 1,076 reads/s 2,502 reads/s 2.33x 60.0 ms 24.3 ms 59.4%

After the fix, Lance reached 2,463–2,531 reads/s with 23.9–24.6 ms p50 latency, matching direct S3 ranged reads at 2,320–2,487 reads/s and 23.8–25.1 ms p50. Every sample remained one GET with zero HEAD requests, and physical bytes matched logical bytes.

The benchmark harness used to establish these results is intentionally not included in this pull request.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Blob read concurrency

Layer / File(s) Summary
Asynchronous blob batch dispatch
rust/lance/src/dataset/blob.rs
BlobSource::drain_pending_reads spawns fulfillment tasks, allowing subsequent batches to dispatch without awaiting earlier batches.
Range request tracking and gating
rust/lance/src/dataset/blob.rs
RecordingRangeObjectStore tracks started, active, and peak requests with atomics and gates range reads with a semaphore.
Concurrency test coordination and assertions
rust/lance/src/dataset/blob.rs
Blob concurrency tests use semaphore permits and atomic tracking to validate staggered requests and ready-plan emission.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BlobSource
  participant TokioTask
  participant FileScheduler
  BlobSource->>TokioTask: Spawn blob batch fulfillment
  TokioTask->>FileScheduler: Dispatch current batch
  BlobSource->>TokioTask: Dispatch subsequent batch
  FileScheduler-->>TokioTask: Apply scheduling and backpressure
Loading

Suggested reviewers: wjones127

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: dispatching concurrent blob range reads.
Description check ✅ Passed The description matches the patch and explains the concurrency fix and test coverage.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch xuanwo/fix-blob-range-concurrency

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot added the bug Something isn't working label Jul 20, 2026
@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@Xuanwo
Xuanwo marked this pull request as ready for review July 20, 2026 15:40
@Xuanwo
Xuanwo merged commit 20992ae into main Jul 20, 2026
33 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/fix-blob-range-concurrency branch July 20, 2026 17:47
wjones127 pushed a commit that referenced this pull request Jul 21, 2026
Concurrent BlobFile range reads against the same physical source were
serialized between opportunistic batches because BlobSource waited for
each batch to finish I/O before draining requests that arrived while it
was in flight. This made caller concurrency scale poorly even though the
Python binding releases the GIL and the existing scheduler can execute
the reads concurrently.

Dispatch each drained batch without waiting for earlier batches to
finish, while leaving FileScheduler responsible for I/O concurrency and
byte backpressure. Deterministic same-source and multi-source gated
tests cover the batching boundary and existing order and cursor
behavior.

### Real S3 impact

Measured on Amazon S3 in us-east-2 with 100 KiB range reads at
concurrency 64:

| Configuration | Before | After | Throughput gain | p50 before | p50
after | p50 reduction |
| --- | ---: | ---: | ---: | ---: | ---: | ---: |
| Dedicated / same source | 876 reads/s | 2,531 reads/s | 2.89x | 64.7
ms | 23.9 ms | 63.1% |
| Dedicated / multiple sources | 1,169 reads/s | 2,505 reads/s | 2.14x |
53.5 ms | 24.0 ms | 55.1% |
| Packed / same source | 1,004 reads/s | 2,463 reads/s | 2.45x | 59.4 ms
| 24.6 ms | 58.5% |
| Packed / multiple sources | 1,076 reads/s | 2,502 reads/s | 2.33x |
60.0 ms | 24.3 ms | 59.4% |

After the fix, Lance reached 2,463–2,531 reads/s with 23.9–24.6 ms p50
latency, matching direct S3 ranged reads at 2,320–2,487 reads/s and
23.8–25.1 ms p50. Every sample remained one GET with zero HEAD requests,
and physical bytes matched logical bytes.

The benchmark harness used to establish these results is intentionally
not included in this pull request.

(cherry picked from commit 20992ae)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants