HBASE-30403 CustomDateTieredCompactionPolicy#getCompactBoundariesForMajor drops the cutOffTimestamp boundary when files lack CUSTOM_TIERING_TIME_RANGE

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<Long> getCompactBoundariesForMajor(Collection<HStoreFile> 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<Long> 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;
   }
 

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<HStoreFile> files = new ArrayList<>();
     files.add(createFile(file, 0, EnvironmentEdgeManager.currentTime(), 1024, 0));
-    assertEquals(3,
+    assertEquals(2,
       ((DateTieredCompactionRequest) policy.selectMajorCompaction(files)).getBoundaries().size());
   }
 

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,16 +236,17 @@ 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 -> {
         byte[] rangeBytes = file.getMetadataValue(CUSTOM_TIERING_TIME_RANGE);
         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,20 +280,17 @@ 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 -> {
         byte[] rangeBytes = file.getMetadataValue(CUSTOM_TIERING_TIME_RANGE);
         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());
         }