perf(common): avoid redundant V1 archived timeline loads in incremental queries - #19553
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19553 +/- ##
=========================================
Coverage 76.84% 76.84%
- Complexity 32379 32391 +12
=========================================
Files 2522 2522
Lines 139106 139154 +48
Branches 16713 16720 +7
=========================================
+ Hits 106892 106930 +38
- Misses 24621 24631 +10
Partials 7593 7593
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
hudi-agent
left a comment
hudi-agent
left a comment
There was a problem hiding this comment.
Thanks for the contribution! This PR splits IncrementalQueryAnalyzer.analyze into dedicated V1 and V2 paths so that V1 resolves incremental ranges by applying InstantRange directly to requestedTime and loads the filtered archived timeline at most once, avoiding the redundant second archive scan. I traced the new V1 branches against the V2 reference semantics in CompletionTimeQueryViewV2.getInstantTimes and the shared createQueryContext, and the behavior appears preserved. No correctness issues found. A few style/readability suggestions in the inline comments. Please take a look, and this should be ready for a Hudi committer or PMC member to take it from here. - a Hudi committer or PMC member can take it from here for a final review.
. A few naming and structural suggestions below, but overall the code is clean and well-organized.
cc @yihua
| } | ||
|
|
||
| @Test | ||
| void testV1ActiveOnlyAndSnapshotRangesDoNotLoadArchive() { |
There was a problem hiding this comment.
🤖 nit: this test covers two distinct cases (active-only range and earliest-snapshot range) in one method, which makes it harder to pinpoint which scenario caused a failure. Could you split it into testV1ActiveOnlyRangeDoesNotLoadArchive and testV1EarliestSnapshotDoesNotLoadArchive?
|
this is the potential improvement mentioned in #19338, please help review, thanks! @danny0405 |
4 participants
Describe the issue this Pull Request addresses
This is a follow-up to #19338 and addresses the archived-timeline optimization suggested in #19338 (comment).
For timeline layout V1,
CompletionTimeQueryViewV1loaded archived instants to identify candidate requested times, after whichIncrementalQueryAnalyzerdiscarded that timeline and loaded the archive again to constructQueryContext. Since V1 completion time is equivalent to requested time, the second scan is redundant.Summary and Changelog
IncrementalQueryAnalyzer.InstantRangedirectly toHoodieInstant.requestedTime()and reuse the same archived timeline inQueryContext.Impact
Improves incremental-query performance for timeline layout V1 (table versions 5–7) by eliminating a redundant archived-timeline scan. There are no public API, configuration, storage-format, or V2 semantic changes. Query results and existing range semantics remain unchanged.
Risk Level
low
The change touches V1 incremental range selection but keeps V2 on the existing path. It is covered by
TestIncrementalQueryAnalyzerandTestCompletionTimeQueryViewV1: 16 tests passed with no failures or errors. Reactor compilation, Checkstyle, RAT, andgit diff --checkalso passed.Documentation Update
none. This is an internal performance optimization with no new user-facing behavior or configuration.
Contributor's checklist