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..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,38 +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); - 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); - } - } - 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 (min.getValue() < cutOffTimestamp) { - boundaries.add(min.getValue()); - if (max.getValue() > cutOffTimestamp) { - boundaries.add(cutOffTimestamp); - } - } + boundaries.add(cutOffTimestamp); return boundaries; } 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()); }