fix(hbase): handle missing CUSTOM_TIERING_TIME_RANGE in getCompactBoundariesForMajor - #8573
fix(hbase): handle missing CUSTOM_TIERING_TIME_RANGE in getCompactBoundariesForMajor#8573shoemoney wants to merge 2 commits into
Conversation
…ndariesForMajor Fix verified RED->GREEN. getCompactBoundariesForMajor silently drops files lacking CUSTOM_TIERING_TIME_RANGE at CustomDateTieredCompactionPolicy.java:72
wchevreuil
left a comment
There was a problem hiding this comment.
Yes, currently, if no file in the list has the CUSTOM_TIERING_TIME_RANGE tag, getCompactBoundariesForMajor returns a single MIN boundary, and a single file will result from the compaction. This single resulting file will now have the CUSTOM_TIERING_TIME_RANGE tag, and a subsequent compaction would be able to define two boundaries, if the time range cross the cutOffTimestamp. This is not optimal, this fix would solve this, but after reviewing this getCompactBoundariesForMajor and the append logic in CustomTieringMultiFileWriter, I think we can simply always set the min and cutOffTimestamp boundaries without needing to traverse the files to check the CUSTOM_TIERING_TIME_RANGE.
Please open a jira ticket to link this PR.
| long now) { | ||
| MutableLong min = new MutableLong(Long.MAX_VALUE); | ||
| MutableLong max = new MutableLong(0); | ||
| boolean[] hasMissing = new boolean[1]; |
There was a problem hiding this comment.
No need to declare an array here.
… cutOffTimestamp boundary CustomTieringMultiFileWriter#append already routes each cell to its tier by comparing against the returned boundaries and skips committing a file for a tier that receives no data, so traversing filesToCompact to inspect CUSTOM_TIERING_TIME_RANGE is unnecessary. Always returning [MIN_VALUE, cutOffTimestamp] is simpler and does not miss the boundary when a file lacks the metadata.
|
Removed the boolean[] array and simplified: getCompactBoundariesForMajor now always returns [MIN_VALUE, cutOffTimestamp] without traversing filesToCompact, since CustomTieringMultiFileWriter#append already buckets by boundary and skips committing empty tiers. Fixed in 7637d74. |
Thank you very much for reporting and addressing this. Before we can merge this, we need a related jira ticket created under https://issues.apache.org/jira/projects/HBASE. If you don't have a jira account yet, you can request one at https://selfserve.apache.org/jira-account.html. Please make sure to select hbase as the 'ASF project you want to file a ticket' so we can receive your request and process it. |
Bug: getCompactBoundariesForMajor silently drops files lacking CUSTOM_TIERING_TIME_RANGE, causing compaction boundaries to miss the cutOffTimestamp split.
Fix: getCompactBoundariesForMajor now unconditionally returns [MIN_VALUE, cutOffTimestamp] without traversing filesToCompact. This is safe because CustomTieringMultiFileWriter#append already buckets each cell into its tier by comparing against these boundaries directly, and only commits a file for a tier that actually received data. Always offering the cutOffTimestamp boundary avoids missing it when a file lacks the metadata.
Verified: existing compaction tests pass. Single file changed: CustomDateTieredCompactionPolicy.java (removed the min/max traversal and MutableLong tracking, replaced with a short comment explaining the simplification).