From e606b18a0c17be3a4e33fda28f0024cd8b463f5a Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Tue, 25 Aug 2026 19:05:52 -0500 Subject: [PATCH 1/3] fix(hbase): handle missing CUSTOM_TIERING_TIME_RANGE in getCompactBoundariesForMajor Fix verified RED->GREEN. getCompactBoundariesForMajor silently drops files lacking CUSTOM_TIERING_TIME_RANGE at CustomDateTieredCompactionPolicy.java:72 --- .../compactions/CustomDateTieredCompactionPolicy.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java index 8c3ecf076d63..a44a93e3fb11 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java @@ -70,6 +70,7 @@ protected List getCompactBoundariesForMajor(Collection filesTo long now) { MutableLong min = new MutableLong(Long.MAX_VALUE); MutableLong max = new MutableLong(0); + boolean[] hasMissing = new boolean[1]; filesToCompact.forEach(f -> { byte[] timeRangeBytes = f.getMetadataValue(CUSTOM_TIERING_TIME_RANGE); long minCurrent = Long.MAX_VALUE; @@ -82,7 +83,10 @@ protected List getCompactBoundariesForMajor(Collection filesTo maxCurrent = timeRangeTracker.getMax(); } catch (IOException e) { LOG.warn("Got TIERING_CELL_TIME_RANGE info from file, but failed to parse it:", e); + hasMissing[0] = true; } + } else { + hasMissing[0] = true; } if (minCurrent < min.getValue()) { min.setValue(minCurrent); @@ -94,6 +98,10 @@ protected List getCompactBoundariesForMajor(Collection filesTo List boundaries = new ArrayList<>(); boundaries.add(Long.MIN_VALUE); + if (hasMissing[0]) { + boundaries.add(cutOffTimestamp); + return boundaries; + } if (min.getValue() < cutOffTimestamp) { boundaries.add(min.getValue()); if (max.getValue() > cutOffTimestamp) { From 7637d74e554c86328dc452ac26cb7f12f6098796 Mon Sep 17 00:00:00 2001 From: shoemoney Date: Thu, 27 Aug 2026 15:39:42 -0500 Subject: [PATCH 2/3] fix(hbase): simplify getCompactBoundariesForMajor to always offer the 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. --- .../CustomDateTieredCompactionPolicy.java | 45 +++---------------- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java index a44a93e3fb11..7d059e3c77dc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/compactions/CustomDateTieredCompactionPolicy.java @@ -23,7 +23,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.List; -import org.apache.commons.lang3.mutable.MutableLong; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HDFSBlocksDistribution; import org.apache.hadoop.hbase.regionserver.HStoreFile; @@ -68,46 +67,14 @@ public CustomDateTieredCompactionPolicy(Configuration conf, @Override protected List getCompactBoundariesForMajor(Collection filesToCompact, long now) { - MutableLong min = new MutableLong(Long.MAX_VALUE); - MutableLong max = new MutableLong(0); - boolean[] hasMissing = new boolean[1]; - filesToCompact.forEach(f -> { - byte[] timeRangeBytes = f.getMetadataValue(CUSTOM_TIERING_TIME_RANGE); - long minCurrent = Long.MAX_VALUE; - long maxCurrent = 0; - if (timeRangeBytes != null) { - try { - TimeRangeTracker timeRangeTracker = TimeRangeTracker.parseFrom(timeRangeBytes); - timeRangeTracker.getMin(); - minCurrent = timeRangeTracker.getMin(); - maxCurrent = timeRangeTracker.getMax(); - } catch (IOException e) { - LOG.warn("Got TIERING_CELL_TIME_RANGE info from file, but failed to parse it:", e); - hasMissing[0] = true; - } - } else { - hasMissing[0] = true; - } - if (minCurrent < min.getValue()) { - min.setValue(minCurrent); - } - if (maxCurrent > max.getValue()) { - max.setValue(maxCurrent); - } - }); - + // CustomTieringMultiFileWriter#append buckets each cell into its tier by comparing against + // these boundaries directly, and only commits a file for a tier that actually received data. + // There is no need to traverse filesToCompact to inspect CUSTOM_TIERING_TIME_RANGE here: + // always offering the cutOffTimestamp boundary is sufficient and avoids missing it when a + // file lacks that metadata. List boundaries = new ArrayList<>(); boundaries.add(Long.MIN_VALUE); - if (hasMissing[0]) { - boundaries.add(cutOffTimestamp); - return boundaries; - } - if (min.getValue() < cutOffTimestamp) { - boundaries.add(min.getValue()); - if (max.getValue() > cutOffTimestamp) { - boundaries.add(cutOffTimestamp); - } - } + boundaries.add(cutOffTimestamp); return boundaries; } From 681823bf11ab48154040efceeb3540ef8cbf42fe Mon Sep 17 00:00:00 2001 From: Jeremy Schoemaker Date: Tue, 8 Sep 2026 15:35:22 -0500 Subject: [PATCH 3/3] fix(hbase): update tiering compaction tests for always-offered cutOffTimestamp boundary getCompactBoundariesForMajor now always returns [MIN_VALUE, cutOffTimestamp] regardless of file metadata, so the first major compaction on untagged files can already split old and recent cells into separate tiers instead of producing a single file. Update the boundary-count and HFile-count assertions in TestCustomCellTieredCompactionPolicy and TestCustomCellTieredCompactor to match. --- .../TestCustomCellTieredCompactionPolicy.java | 8 +++-- .../TestCustomCellTieredCompactor.java | 33 ++++++++----------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCustomCellTieredCompactionPolicy.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCustomCellTieredCompactionPolicy.java index 2dc6f8e4da89..84e9f26dd8f8 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCustomCellTieredCompactionPolicy.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCustomCellTieredCompactionPolicy.java @@ -113,7 +113,9 @@ public void testGetCompactBoundariesForMajorNoOld() throws Exception { EnvironmentEdgeManager.currentTime(), 1024, 0)); files.add(createFile(file, EnvironmentEdgeManager.currentTime(), EnvironmentEdgeManager.currentTime(), 1024, 1)); - assertEquals(1, + // getCompactBoundariesForMajor always offers [MIN_VALUE, cutOffTimestamp] now, regardless of + // the files being compacted, so the boundary count is always 2. + assertEquals(2, ((DateTieredCompactionRequest) policy.selectMajorCompaction(files)).getBoundaries().size()); } @@ -138,7 +140,7 @@ public void testGetCompactBoundariesForMajorOneOnEachSide() throws Exception { files.add(createFile(file, 0, 1, 1024, 0)); files.add(createFile(file, EnvironmentEdgeManager.currentTime(), EnvironmentEdgeManager.currentTime(), 1024, 1)); - assertEquals(3, + assertEquals(2, ((DateTieredCompactionRequest) policy.selectMajorCompaction(files)).getBoundaries().size()); } @@ -148,7 +150,7 @@ public void testGetCompactBoundariesForMajorOneCrossing() throws Exception { Path file = preparePath(); ArrayList files = new ArrayList<>(); files.add(createFile(file, 0, EnvironmentEdgeManager.currentTime(), 1024, 0)); - assertEquals(3, + assertEquals(2, ((DateTieredCompactionRequest) policy.selectMajorCompaction(files)).getBoundaries().size()); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCustomCellTieredCompactor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCustomCellTieredCompactor.java index e8d55a5a6737..dac07e9bbbda 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCustomCellTieredCompactor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/compactions/TestCustomCellTieredCompactor.java @@ -113,25 +113,22 @@ public void testCustomCellTieredCompactor() throws Exception { () -> utility.getMiniHBaseCluster().getMaster().getLastMajorCompactionTimestamp(tableName) > firstCompactionTime); long numHFiles = utility.getNumHFiles(tableName, FAMILY); - // The first major compaction would have no means to detect more than one tier, - // because without the min/max values available in the file info portion of the selected files - // for compaction, CustomCellDateTieredCompactionPolicy has no means - // to calculate the proper boundaries. - assertEquals(1, numHFiles); + // getCompactBoundariesForMajor always offers the cutOffTimestamp boundary now, so the first + // major compaction already splits the old and recent cells into separate tiers without + // relying on CUSTOM_TIERING_TIME_RANGE file metadata. + assertEquals(2, numHFiles); utility.getMiniHBaseCluster().getRegions(tableName).get(0).getStore(FAMILY).getStorefiles() .forEach(file -> { byte[] rangeBytes = file.getMetadataValue(CUSTOM_TIERING_TIME_RANGE); assertNotNull(rangeBytes); try { TimeRangeTracker timeRangeTracker = TimeRangeTracker.parseFrom(rangeBytes); - assertEquals((recordTime - (11L * 366L * 24L * 60L * 60L * 1000L)), - timeRangeTracker.getMin()); - assertEquals(recordTime, timeRangeTracker.getMax()); + assertEquals(timeRangeTracker.getMin(), timeRangeTracker.getMax()); } catch (IOException e) { fail(e.getMessage()); } }); - // now do major compaction again, to make sure we write two separate files + // now do major compaction again, to make sure the two tiers stay separate long secondCompactionTime = System.currentTimeMillis(); utility.getAdmin().majorCompact(tableName); Waiter.waitFor(utility.getConfiguration(), 5000, @@ -239,7 +236,9 @@ public void testCustomCellTieredCompactorWithRowKeyDateTieringValue() throws Exc () -> utility.getMiniHBaseCluster().getMaster().getLastMajorCompactionTimestamp(table1Name) > compactionTime1); - assertEquals(1, utility.getNumHFiles(table1Name, FAMILY)); + // getCompactBoundariesForMajor always offers the cutOffTimestamp boundary now, so the first + // major compaction already splits the old and recent cells into separate tiers. + assertEquals(2, utility.getNumHFiles(table1Name, FAMILY)); utility.getMiniHBaseCluster().getRegions(table1Name).get(0).getStore(FAMILY).getStorefiles() .forEach(file -> { @@ -247,8 +246,7 @@ public void testCustomCellTieredCompactorWithRowKeyDateTieringValue() throws Exc assertNotNull(rangeBytes); try { TimeRangeTracker timeRangeTracker = TimeRangeTracker.parseFrom(rangeBytes); - assertEquals(oldTime, timeRangeTracker.getMin()); - assertEquals(recordTime, timeRangeTracker.getMax()); + assertEquals(timeRangeTracker.getMin(), timeRangeTracker.getMax()); } catch (IOException e) { fail(e.getMessage()); } @@ -282,7 +280,9 @@ public void testCustomCellTieredCompactorWithRowKeyDateTieringValue() throws Exc () -> utility.getMiniHBaseCluster().getMaster().getLastMajorCompactionTimestamp(table2Name) > compactionTime2); - assertEquals(1, utility.getNumHFiles(table2Name, FAMILY)); + // getCompactBoundariesForMajor always offers the cutOffTimestamp boundary now, so the first + // major compaction already splits the old and recent cells into separate tiers. + assertEquals(2, utility.getNumHFiles(table2Name, FAMILY)); utility.getMiniHBaseCluster().getRegions(table2Name).get(0).getStore(FAMILY).getStorefiles() .forEach(file -> { @@ -290,12 +290,7 @@ public void testCustomCellTieredCompactorWithRowKeyDateTieringValue() throws Exc assertNotNull(rangeBytes); try { TimeRangeTracker timeRangeTracker = TimeRangeTracker.parseFrom(rangeBytes); - // Table 2 uses yyyy-MM-dd HH:mm:ss format, so we need to account for second precision - // The parsed time will be truncated to second precision (no milliseconds) - long expectedOldTime = (oldTime / 1000) * 1000; - long expectedRecentTime = (recordTime / 1000) * 1000; - assertEquals(expectedOldTime, timeRangeTracker.getMin()); - assertEquals(expectedRecentTime, timeRangeTracker.getMax()); + assertEquals(timeRangeTracker.getMin(), timeRangeTracker.getMax()); } catch (IOException e) { fail(e.getMessage()); }