You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "Dandandan (via GitHub)" <gi...@apache.org> on 2026/06/22 22:03:15 UTC
[PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Dandandan opened a new pull request, #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777
`LocalFileSystem::get_ranges` issued one read per range. This coalesces contiguous/overlapping ranges into a single read, collapsing many adjacent ranges into far fewer syscalls.
- Uses a coalesce gap of **0** (vs the 1 MiB network default), so we never read bytes that weren't requested — important for a page-cache-backed file where an extra `pread` is cheap but wasted bytes are not.
- A **fast path** skips the coalescing bookkeeping entirely when nothing merges, so disjoint/sparse workloads are unaffected.
- Reuses `util::merge_ranges` (now `pub(crate)`) and the same slice-back logic as `coalesce_ranges`.
On a warm-cache benchmark, many adjacent small ranges drop ~2.6× (e.g. 1024 × 4 KiB contiguous: ~700 µs → ~260 µs); disjoint ranges are unchanged.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan commented on code in PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777#discussion_r3491206889
##########
src/local.rs:
##########
@@ -502,13 +502,39 @@ impl ObjectStore for LocalFileSystem {
let path = self.path_to_filesystem(location)?;
let ranges = ranges.to_vec();
maybe_spawn_blocking(move || {
- // Vectored IO might be faster
Review Comment:
The comment is incorrect, there doesn't exist any "Vectored IO" API for this (well closest would be io_uring)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "Dandandan (via GitHub)" <gi...@apache.org>.
Dandandan merged PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777#discussion_r3492654949
##########
src/local.rs:
##########
@@ -502,13 +502,39 @@ impl ObjectStore for LocalFileSystem {
let path = self.path_to_filesystem(location)?;
let ranges = ranges.to_vec();
maybe_spawn_blocking(move || {
- // Vectored IO might be faster
Review Comment:
oh yea, makes sense 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777#discussion_r3462673105
##########
src/local.rs:
##########
@@ -1739,6 +1765,42 @@ mod tests {
assert_eq!(&*read_data, data);
}
+ #[tokio::test]
+ async fn get_ranges_coalesces() {
+ let root = TempDir::new().unwrap();
+ let integration = LocalFileSystem::new_with_prefix(root.path()).unwrap();
+ let location = Path::from("some_file");
+
+ // Position-dependent content so a misrouted slice is detectable.
+ let data: Bytes = (0..4096u32)
+ .map(|i| (i % 251) as u8)
+ .collect::<Vec<_>>()
+ .into();
+ integration
+ .put(&location, data.clone().into())
+ .await
+ .unwrap();
+
+ let cases: Vec<Vec<Range<u64>>> = vec![
+ vec![0..16, 16..32, 32..48], // adjacent -> coalesced
+ vec![0..16, 64..80, 1000..1016], // gapped -> not coalesced
+ vec![100..120, 0..50, 40..60, 0..50, 110..130], // out-of-order, overlapping, duplicate
+ vec![], // empty
+ ];
+
+ for ranges in cases {
+ let got = integration.get_ranges(&location, &ranges).await.unwrap();
Review Comment:
I don't think this test verifies that the underlying requests were coalesced -- is it possible to do that? Like I think these tests pass even if we revert all the code changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777#discussion_r3475799405
##########
src/local.rs:
##########
@@ -502,13 +502,39 @@ impl ObjectStore for LocalFileSystem {
let path = self.path_to_filesystem(location)?;
let ranges = ranges.to_vec();
maybe_spawn_blocking(move || {
- // Vectored IO might be faster
Review Comment:
worth keeping this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Coalesce contiguous ranges in `LocalFileSystem::get_ranges` [arrow-rs-object-store]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #777:
URL: https://github.com/apache/arrow-rs-object-store/pull/777#discussion_r3548008736
##########
src/local.rs:
##########
@@ -1739,6 +1765,42 @@ mod tests {
assert_eq!(&*read_data, data);
}
+ #[tokio::test]
+ async fn get_ranges_coalesces() {
+ let root = TempDir::new().unwrap();
+ let integration = LocalFileSystem::new_with_prefix(root.path()).unwrap();
+ let location = Path::from("some_file");
+
+ // Position-dependent content so a misrouted slice is detectable.
+ let data: Bytes = (0..4096u32)
+ .map(|i| (i % 251) as u8)
+ .collect::<Vec<_>>()
+ .into();
+ integration
+ .put(&location, data.clone().into())
+ .await
+ .unwrap();
+
+ let cases: Vec<Vec<Range<u64>>> = vec![
+ vec![0..16, 16..32, 32..48], // adjacent -> coalesced
+ vec![0..16, 64..80, 1000..1016], // gapped -> not coalesced
+ vec![100..120, 0..50, 40..60, 0..50, 110..130], // out-of-order, overlapping, duplicate
+ vec![], // empty
+ ];
+
+ for ranges in cases {
+ let got = integration.get_ranges(&location, &ranges).await.unwrap();
Review Comment:
(sorry I forgot to click submit on the PR)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org