[
https://issues.apache.org/jira/browse/ORC-2223?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
yongzhi.shao updated ORC-2223:
------------------------------
Description:
Description:
h3. Problem
When scanning an ORC file, {{RecordReaderImpl.beginReadStripe()}} already reads
the {{OrcProto.StripeFooter}} for the current stripe (via
{{{}readStripeFooter(stripe){}}}). Immediately afterwards,
{{RecordReaderImpl.readStripe()}} calls {{{}StripePlanner.parseStripe(stripe,
fileIncluded){}}}, which internally issues a second, identical
{{DataReader.readStripeFooter(stripe)}} call for the same stripe.
This means every stripe during a scan performs a duplicate footer read (and
protobuf re-parse) from the underlying {{{}DataReader{}}}. For remote/object
stores or high-stripe-count files, this is a measurable, avoidable source of
extra I/O and CPU, and it scales linearly with the number of stripes read.
h3. Root cause
{{StripePlanner.parseStripe(StripeInformation, boolean[])}} has a single entry
point that unconditionally reads the footer itself, and {{RecordReaderImpl}}
does not pass along the footer it already obtained in {{{}beginReadStripe(){}}}.
h3. Proposed fix
* Add an overloaded {{StripePlanner.parseStripe(StripeInformation, boolean[],
OrcProto.StripeFooter)}} that accepts a footer already read by the caller. The
existing two-argument {{parseStripe}} remains as a thin wrapper that reads the
footer itself, preserving compatibility for other callers.
* In {{{}RecordReaderImpl.readStripe(){}}}, pass the footer already obtained
in {{beginReadStripe()}} to the new overload, eliminating one
{{readStripeFooter}} call per stripe.
h3. Scope / compatibility
* No change to the ORC file format or wire behavior.
* Public API is preserved (old {{parseStripe}} signature retained); no
breaking changes.
* No C++ reader changes needed, this duplication is specific to the Java
reader implementation.
h3. Testing
* New regression test
{{{}TestRecordReaderImpl.testNoDuplicateStripeFooterRead{}}}: uses a Mockito
spy on {{DataReader}} (with {{clone()}} stubbed to return the spy, since
{{RecordReaderImpl}} clones the supplied reader) to assert exactly one
{{readStripeFooter}} call per stripe, for both a plain scan and an
SArg-filtered scan.
* Lowered read-percentage thresholds in {{{}TestMinSeekSize{}}},
{{{}TestRowFilteringComplexTypesNulls{}}}, and {{TestRowFilteringIOSkip}} to
reflect the genuinely reduced bytes read per stripe.
* See : [ORC-2223 : [Java] RecordReaderImpl reads and parses each StripeFooter
twice per stripe during scans|https://github.com/apache/orc/pull/2717]
was:
Description:
h3. Problem
When scanning an ORC file, \{{RecordReaderImpl.beginReadStripe()}} already
reads the \{{OrcProto.StripeFooter}} for the current stripe (via
\{{readStripeFooter(stripe)}}). Immediately afterwards,
\{{RecordReaderImpl.readStripe()}} calls \{{StripePlanner.parseStripe(stripe,
fileIncluded)}}, which internally issues a second, identical
\{{DataReader.readStripeFooter(stripe)}} call for the same stripe.
This means every stripe during a scan performs a duplicate footer read (and
protobuf re-parse) from the underlying \{{DataReader}}. For remote/object
stores or high-stripe-count files, this is a measurable, avoidable source of
extra I/O and CPU, and it scales linearly with the number of stripes read.
h3. Root cause
{\{StripePlanner.parseStripe(StripeInformation, boolean[])}} has a single entry
point that unconditionally reads the footer itself, and \{{RecordReaderImpl}}
does not pass along the footer it already obtained in \{{beginReadStripe()}}.
h3. Proposed fix
* Add an overloaded \{{StripePlanner.parseStripe(StripeInformation, boolean[],
OrcProto.StripeFooter)}} that accepts a footer already read by the caller. The
existing two-argument \{{parseStripe}} remains as a thin wrapper that reads the
footer itself, preserving compatibility for other callers.
* In \{{RecordReaderImpl.readStripe()}}, pass the footer already obtained in
\{{beginReadStripe()}} to the new overload, eliminating one
\{{readStripeFooter}} call per stripe.
h3. Scope / compatibility
* No change to the ORC file format or wire behavior.
* Public API is preserved (old \{{parseStripe}} signature retained); no
breaking changes.
* No C++ reader changes needed, this duplication is specific to the Java reader
implementation.
h3. Testing
* New regression test
\{{TestRecordReaderImpl.testNoDuplicateStripeFooterRead}}: uses a Mockito spy
on \{{DataReader}} (with \{{clone()}} stubbed to return the spy, since
\{{RecordReaderImpl}} clones the supplied reader) to assert exactly one
\{{readStripeFooter}} call per stripe, for both a plain scan and an
SArg-filtered scan.
* Lowered read-percentage thresholds in \{{TestMinSeekSize}},
\{{TestRowFilteringComplexTypesNulls}}, and \{{TestRowFilteringIOSkip}} to
reflect the genuinely reduced bytes read per stripe.
> [Java] RecordReaderImpl reads and parses each StripeFooter twice per stripe
> during scans
> ----------------------------------------------------------------------------------------
>
> Key: ORC-2223
> URL: https://issues.apache.org/jira/browse/ORC-2223
> Project: ORC
> Issue Type: Improvement
> Reporter: yongzhi.shao
> Priority: Major
>
> Description:
> h3. Problem
> When scanning an ORC file, {{RecordReaderImpl.beginReadStripe()}} already
> reads the {{OrcProto.StripeFooter}} for the current stripe (via
> {{{}readStripeFooter(stripe){}}}). Immediately afterwards,
> {{RecordReaderImpl.readStripe()}} calls {{{}StripePlanner.parseStripe(stripe,
> fileIncluded){}}}, which internally issues a second, identical
> {{DataReader.readStripeFooter(stripe)}} call for the same stripe.
> This means every stripe during a scan performs a duplicate footer read (and
> protobuf re-parse) from the underlying {{{}DataReader{}}}. For remote/object
> stores or high-stripe-count files, this is a measurable, avoidable source of
> extra I/O and CPU, and it scales linearly with the number of stripes read.
> h3. Root cause
> {{StripePlanner.parseStripe(StripeInformation, boolean[])}} has a single
> entry point that unconditionally reads the footer itself, and
> {{RecordReaderImpl}} does not pass along the footer it already obtained in
> {{{}beginReadStripe(){}}}.
> h3. Proposed fix
> * Add an overloaded {{StripePlanner.parseStripe(StripeInformation,
> boolean[], OrcProto.StripeFooter)}} that accepts a footer already read by the
> caller. The existing two-argument {{parseStripe}} remains as a thin wrapper
> that reads the footer itself, preserving compatibility for other callers.
> * In {{{}RecordReaderImpl.readStripe(){}}}, pass the footer already obtained
> in {{beginReadStripe()}} to the new overload, eliminating one
> {{readStripeFooter}} call per stripe.
> h3. Scope / compatibility
> * No change to the ORC file format or wire behavior.
> * Public API is preserved (old {{parseStripe}} signature retained); no
> breaking changes.
> * No C++ reader changes needed, this duplication is specific to the Java
> reader implementation.
> h3. Testing
> * New regression test
> {{{}TestRecordReaderImpl.testNoDuplicateStripeFooterRead{}}}: uses a Mockito
> spy on {{DataReader}} (with {{clone()}} stubbed to return the spy, since
> {{RecordReaderImpl}} clones the supplied reader) to assert exactly one
> {{readStripeFooter}} call per stripe, for both a plain scan and an
> SArg-filtered scan.
> * Lowered read-percentage thresholds in {{{}TestMinSeekSize{}}},
> {{{}TestRowFilteringComplexTypesNulls{}}}, and {{TestRowFilteringIOSkip}} to
> reflect the genuinely reduced bytes read per stripe.
> * See : [ORC-2223 : [Java] RecordReaderImpl reads and parses each
> StripeFooter twice per stripe during
> scans|https://github.com/apache/orc/pull/2717]
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
