Core: Coalesce nearby blob reads in PuffinReader.readAll - #17418
Open
vishnuprakaz wants to merge 1 commit into
Open
Core: Coalesce nearby blob reads in PuffinReader.readAll#17418vishnuprakaz wants to merge 1 commit into
vishnuprakaz wants to merge 1 commit into
Conversation
Resolve the TODO from apache#4537 by reading contiguous blobs in a single request, handed back as no-copy ByteBuffer views. A contiguous run is split into bounded reads (capped at MANIFEST_TARGET_SIZE, 8 MiB) so a large blob section never allocates one oversized buffer.
vishnuprakaz
force-pushed
the
puffin-readall-coalesce
branch
from
July 29, 2026 16:45
8f46a60 to
b663983
Compare
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
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.
PuffinReader.readAllcurrently issues oneseek+ read per blob. This resolvesthe long-standing TODO from #4537:
Contiguous (or overlapping) blobs are now read in a single request and handed back
as no-copy ByteBuffer views over that region. Since Puffin stats/index blobs are
written contiguously, a readAll over many blobs collapses from N reads to one
the win matters most on object stores, where each read is a round trip.
Approach
This coalesces contiguous blob reads: after sorting by offset, adjacent/overlapping blobs are merged into a single read, and a run is split once it would exceed a max read size.
Also
RangeReadable.readVectored(...)was considered as an alternative, but its default implementation doesn't coalesce on S3/Hadoop (only some FileIOs, e.g. GCS analytics, optimize it) and eagerly buffers every range.Coalescing contiguous reads captures the benefit for real Puffin layouts without reading bytes the caller didn't request, and reads stay lazy (a region is fetched only when iterated).
The max read size is capped at 8 MiB the same value as
MANIFEST_TARGET_SIZE, since Puffin blobs are metadata. The cap bounds the extra memory coalescing holds compared to the original per-blob reads (a single blob larger than the cap is still read on its own).Are these changes tested?
readAll end-to-end for both codecs (contiguous blobs, nonzero-offset slice).
separate reads.
split, and an oversized single blob kept in its own read.
Are there any user-facing changes?
No