Skip to content

fix(object_store): make get_ranges coalescing configurable - #8062

Open
Xuanwo wants to merge 1 commit into
mainfrom
xuanwo/object-store-get-ranges-fetch
Open

fix(object_store): make get_ranges coalescing configurable#8062
Xuanwo wants to merge 1 commit into
mainfrom
xuanwo/object-store-get-ranges-fetch

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Aug 13, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

Follow-up to #7380 and #7383.

Rationale for this change

object_store_opendal::get_ranges currently executes one reader.read(range) for every requested range. The reads are buffered concurrently, but nearby ranges still cause separate backend reads.

Reader::fetch already provides range coalescing and concurrent execution. However, the earlier HDFS investigation in #7380 found that unconditional 1 MiB coalescing could approximately double cluster memory usage because small returned Bytes slices retained large coalesced backing buffers. The integration therefore needs a configurable coalescing threshold instead of choosing between always coalescing and never coalescing.

What changes are included in this PR?

  • Route get_ranges through Reader::fetch with the existing concurrency of 8.
  • Default the coalescing gap to object_store::OBJECT_STORE_COALESCE_DEFAULT (1 MiB).
  • Add OpendalStore::with_get_ranges_gap so users can tune the threshold or set it to 0.
  • Make gap = 0 a valid OpenDAL reader setting. It disables merging ranges separated by bytes while still merging overlapping or adjacent ranges.
  • Add regression coverage for the default, disabled, and custom gap behaviors.

Are there any user-facing changes?

Yes. get_ranges now coalesces nearby ranges by default. HDFS users whose workloads are sensitive to retained coalesced buffers can construct the store with .with_get_ranges_gap(0) or choose an intermediate threshold.

Validation

  • cargo test --manifest-path integrations/object_store/Cargo.toml
  • cargo clippy --manifest-path integrations/object_store/Cargo.toml --all-targets --all-features -- -D warnings
  • cargo test -p opendal-core -p opendal-layer-timeout test_merge_ranges
  • cargo clippy -p opendal-core -p opendal-layer-timeout --all-targets -- -D warnings
  • ./scripts/workspace.py cargo fmt -- --check

A real HDFS/DataFusion Comet cluster retest is pending.

AI Usage Statement

OpenAI Codex (GPT-5) assisted with implementation, tests, and PR preparation.

@Xuanwo

Xuanwo commented Aug 13, 2026

Copy link
Copy Markdown
Member Author

@comphead, could you please retest this PR with the same HDFS/DataFusion Comet workload from #7380 and #7383?

Please compare at least:

  1. The default 1 MiB gap.
  2. .with_get_ranges_gap(0) to preserve exact-range reads.
  3. An intermediate gap if practical for the workload.

The most useful results would be task wall-clock time and cluster memory usage. I am keeping the PR in draft until we understand whether the configurable gap avoids the earlier memory regression while improving the HDFS read path.

@Xuanwo
Xuanwo marked this pull request as ready for review August 13, 2026 05:50
@Xuanwo
Xuanwo requested a review from tisonkun as a code owner August 13, 2026 05:50
@dosubot dosubot Bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Aug 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant