Fix reachMinorInterval() starvation for cold partitions#4179
Open
lintingbin wants to merge 5 commits into
Open
Fix reachMinorInterval() starvation for cold partitions#4179lintingbin wants to merge 5 commits into
lintingbin wants to merge 5 commits into
Conversation
reachMinorInterval() uses table-level lastMinorOptimizingTime which is frequently reset by high-traffic partitions. This causes partitions with fewer small files (below minor trigger file count threshold) to never get minor optimized, even when their files accumulate over time. Add a cross-day fallback: when minorLeastInterval is less than one day and the last minor optimizing happened on a different calendar day, reachMinorInterval() returns true. This ensures every partition gets at least one minor optimization attempt per day. Also add Javadoc to SELF_OPTIMIZING_MINOR_TRIGGER_INTERVAL clarifying that it should be less than one day for the cross-day fallback to work. Closes apache#4055
…to pass spotless check
lintingbin
added a commit
to lintingbin/amoro
that referenced
this pull request
Apr 15, 2026
…-iceberg tables on Iceberg 1.7.x Found while investigating the CI failure in apache#4179. Iceberg 1.7.x introduced a breaking change in HadoopTableOperations.commit(): it now uses reference equality (==) to compare the `base` argument against the cached currentMetadata. Previously, newTableOperations() called ops.current() to obtain the current metadata, but versionAndMetadata() inside commit() may refresh the internal state and return a different object instance. When the two references differ, commit() throws CommitFailedException("Cannot commit changes based on stale table metadata") even though the metadata content is identical, causing table loading to fail. Fix: replace ops.current() with ops.refresh() so that the returned TableMetadata reference is the same object stored in ops' internal cache. When commit() then calls versionAndMetadata(), it finds the version unchanged on disk and returns the same cached reference, satisfying the reference-equality check. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
8 tasks
czy006
pushed a commit
that referenced
this pull request
Apr 16, 2026
…-iceberg tables on Iceberg 1.7.2 (#4182) [AMORO-4163][ams] Fix CommitFailedException when loading legacy mixed-iceberg tables on Iceberg 1.7.x Found while investigating the CI failure in #4179. Iceberg 1.7.x introduced a breaking change in HadoopTableOperations.commit(): it now uses reference equality (==) to compare the `base` argument against the cached currentMetadata. Previously, newTableOperations() called ops.current() to obtain the current metadata, but versionAndMetadata() inside commit() may refresh the internal state and return a different object instance. When the two references differ, commit() throws CommitFailedException("Cannot commit changes based on stale table metadata") even though the metadata content is identical, causing table loading to fail. Fix: replace ops.current() with ops.refresh() so that the returned TableMetadata reference is the same object stored in ops' internal cache. When commit() then calls versionAndMetadata(), it finds the version unchanged on disk and returns the same cached reference, satisfying the reference-equality check. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
lintingbin
force-pushed
the
fix-4055-reachMinorInterval
branch
from
767b166 to
0a43f0e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4179 +/- ##
============================================
- Coverage 29.75% 22.69% -7.06%
+ Complexity 4258 2621 -1637
============================================
Files 677 461 -216
Lines 54744 42563 -12181
Branches 6968 6002 -966
============================================
- Hits 16288 9659 -6629
+ Misses 37246 32065 -5181
+ Partials 1210 839 -371
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@amoro.apache.org list. Thank you for your contributions. |
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
2 participants
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.
What changes were proposed in this pull request?
reachMinorInterval()uses the table-levellastMinorOptimizingTime, which is frequently reset by high-traffic partitions. This causes partitions with fewer small files (below the minor trigger file count threshold) to never get minor optimized, even when their files accumulate over time.Root Cause
The
lastMinorOptimizingTimeis a single table-level timestamp shared across all partitions. When a high-traffic partition triggers minor optimization, the timestamp is reset for the entire table. Cold partitions (with few files, not reachingminor-trigger-file-count) rely onreachMinorInterval()to trigger optimization, but the interval check never passes because the table-level timestamp keeps getting refreshed.Solution
Added a cross-day fallback mechanism in
reachMinorInterval():minorLeastIntervalis less than one day (which is the typical configuration, default is 1 hour), and the last minor optimization happened on a different calendar day,reachMinorInterval()returnstrue.minorLeastInterval < 1 dayavoids semantic conflict when users intentionally set a longer interval (e.g., 2 days).Also added Javadoc to
SELF_OPTIMIZING_MINOR_TRIGGER_INTERVALto document the expected range and the cross-day fallback behavior.How was this patch tested?
Existing
TestOptimizingEvaluatortests pass. The change is minimal and additive — it only adds a fallback path that activates when the normal interval check fails and a day boundary has been crossed.Closes #4055