BsoBird opened a new pull request, #2717:
URL: https://github.com/apache/orc/pull/2717
(java): reuse stripe footer in RecordReaderImpl to avoid duplicate read
- Add an overloaded StripePlanner.parseStripe that accepts an
OrcProto.StripeFooter already read by the caller, keeping the existing two-arg
parseStripe as a thin wrapper that reads the footer itself
(java/core/src/java/org/apache/orc/impl/reader/ StripePlanner.java).
- RecordReaderImpl.readStripe now passes the footer obtained in
beginReadStripe to planner.parseStripe, eliminating one readStripeFooter call
per stripe during scans
(java/core/src/java/org/apache/orc/impl/RecordReaderImpl.java).
- Lower read-percentage thresholds in TestMinSeekSize,
TestRowFilteringComplexTypesNulls, and TestRowFilteringIOSkip to reflect the
reduced bytes read per stripe.
- Add TestRecordReaderImpl.testNoDuplicateStripeFooterRead, a regression
test asserting exactly one footer read per stripe for both plain and
SArg-filtered scans.
<!--
Thanks for sending a pull request! Here are some tips for you:
1. File a JIRA issue first and use it as a prefix of your PR title, e.g.,
`ORC-001: Fix ABC`.
2. Use your PR title to summarize what this PR proposes instead of
describing the problem.
3. Make PR title and description complete because these will be the
permanent commit log.
4. If possible, provide a concise and reproducible example to reproduce
the issue for a faster review.
5. If the PR is unfinished, use GitHub PR Draft feature.
-->
### What changes were proposed in this pull request?
This PR eliminates a duplicate `StripeFooter` read in the Java reader: each
stripe's footer was being read and parsed twice per scan.
`RecordReaderImpl.beginReadStripe()` already reads the
`OrcProto.StripeFooter` for the current stripe, but
`RecordReaderImpl.readStripe()` then called `StripePlanner.parseStripe(stripe,
fileIncluded)`, which read the same footer again internally.
Changes:
1. Add an overloaded `StripePlanner.parseStripe(StripeInformation,
boolean[], OrcProto.StripeFooter)` that accepts a footer already read by the
caller. The existing two-argument `parseStripe` is kept as a thin wrapper that
reads the footer itself, so other callers are unaffected.
2. `RecordReaderImpl.readStripe()` now passes the footer obtained in
`beginReadStripe()` to the new overload, removing one
`DataReader.readStripeFooter` call per stripe.
3. Lowered read-percentage thresholds in `TestMinSeekSize`,
`TestRowFilteringComplexTypesNulls`, and `TestRowFilteringIOSkip` to reflect
the genuinely reduced bytes read per stripe.
### Why are the changes needed?
Every stripe read during a scan performed a redundant footer read (plus
protobuf re-parse) from the underlying `DataReader`. On remote/object stores or
for files with many stripes, this is a measurable and avoidable source of extra
I/O and CPU that scales linearly with the number of stripes read. The fix
removes the duplication without changing any read semantics: both reads
previously fetched the identical footer bytes for the same stripe.
### How was this patch tested?
Added a new regression test
`TestRecordReaderImpl.testNoDuplicateStripeFooterRead`. It installs a Mockito
spy on the `DataReader` (stubbing `clone()` to return the spy, since
`RecordReaderImpl` clones the supplied reader) and verifies `readStripeFooter`
is invoked exactly once per stripe — covering both a plain scan and an
SArg-filtered scan (a predicate that evaluates to YES_NO for every row group,
so `pickRowGroups` is exercised without skipping whole stripes).
Also adjusted the read-percentage assertions in `TestMinSeekSize`,
`TestRowFilteringComplexTypesNulls`, and `TestRowFilteringIOSkip`: those tests
assert lower bounds on bytes read, and the fix legitimately reduces bytes read,
so thresholds were lowered accordingly (e.g. 5.9 -> 5.0, 0.06 -> 0.03) with
comments explaining why.
Run: cd java && ./mvnw test -pl core
### Was this patch authored or co-authored using generative AI tooling?
Yes. co-authored-by: Claude (Anthropic)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
