HBASE-30036 Fix redundant DeleteColumn with empty qualifier skipping DeleteFamily

Author: junegunn

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java

@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import org.apache.hadoop.hbase.ExtendedCell;
+import org.apache.hadoop.hbase.KeyValue;
 import org.apache.hadoop.hbase.PrivateCellUtil;
 import org.apache.hadoop.hbase.regionserver.ScanInfo;
 import org.apache.yetus.audience.InterfaceAudience;
@@ -52,6 +53,12 @@ public MatchCode match(ExtendedCell cell) throws IOException {
       // that overwrites tracker state. Seek past remaining cells for this column/row since
       // they are all covered by the previously tracked delete.
       if (deletes.isRedundantDelete(cell)) {
+        // Skip seeking for deletes with empty qualifier, not to skip a subsequent
+        // DeleteFamily marker that covers other qualifiers. DeleteFamily itself can seek
+        // safely because all remaining empty-qualifier cells are redundant under it.
+        if (cell.getQualifierLength() == 0 && typeByte != KeyValue.Type.DeleteFamily.getCode()) {
+          return MatchCode.SKIP;
+        }
         return columns.getNextRowOrNextColumn(cell);
       }
       trackDelete(cell);

hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java

@@ -94,9 +94,10 @@ public void testSkipsRedundantDeleteMarkers() throws IOException {
     assertRetainDeletes(new Type[] { Type.DeleteFamily, Type.DeleteFamily, Type.DeleteFamily },
       INCLUDE, SEEK_NEXT_COL, SEEK_NEXT_COL);
 
-    // DF + DFV interleaved. DF included, DFV at same ts redundant, older DF redundant.
+    // DF + DFV interleaved. DF included, DFV redundant (SKIP because empty qualifier),
+    // older DF redundant (SEEK_NEXT_COL), older DFV redundant (SKIP).
     assertRetainDeletes(new Type[] { Type.DeleteFamily, Type.DeleteFamilyVersion, Type.DeleteFamily,
-      Type.DeleteFamilyVersion }, INCLUDE, SEEK_NEXT_COL, SEEK_NEXT_COL, SEEK_NEXT_COL);
+      Type.DeleteFamilyVersion }, INCLUDE, SKIP, SEEK_NEXT_COL, SKIP);
 
     // Delete (version) covered by DeleteColumn.
     assertRetainDeletes(new Type[] { Type.DeleteColumn, Type.Delete, Type.Delete, Type.Delete },
@@ -108,17 +109,51 @@ public void testSkipsRedundantDeleteMarkers() throws IOException {
       INCLUDE);
   }
 
+  /**
+   * Redundant column-level deletes with empty qualifier must not seek past a subsequent
+   * DeleteFamily. getKeyForNextColumn treats empty qualifier as "no column" and returns
+   * SEEK_NEXT_ROW, which would skip the DF and all remaining cells in the row.
+   */
+  @Test
+  public void testEmptyQualifierDeleteDoesNotSkipDeleteFamily() throws IOException {
+    byte[] emptyQualifier = HConstants.EMPTY_BYTE_ARRAY;
+
+    // DC(empty) + DC(empty) redundant + DF must still be reachable.
+    assertRetainDeletes(emptyQualifier,
+      new Type[] { Type.DeleteColumn, Type.DeleteColumn, Type.DeleteFamily }, INCLUDE, SKIP,
+      INCLUDE);
+
+    // DC(empty) + Delete(empty) redundant + DF must still be reachable.
+    assertRetainDeletes(emptyQualifier,
+      new Type[] { Type.DeleteColumn, Type.Delete, Type.DeleteFamily }, INCLUDE, SKIP, INCLUDE);
+  }
+
   private void assertRetainDeletes(Type[] types, MatchCode... expected) throws IOException {
     assertRetainDeletes(KeepDeletedCells.FALSE, types, expected);
   }
 
+  private void assertRetainDeletes(byte[] qualifier, Type[] types, MatchCode... expected)
+    throws IOException {
+    assertRetainDeletes(KeepDeletedCells.FALSE, qualifier, types, expected);
+  }
+
   /**
    * Build cells from the given types with decrementing timestamps (same ts for adjacent
    * family-level and column-level types at the same position). Family-level types (DeleteFamily,
    * DeleteFamilyVersion) use empty qualifier; others use col1.
    */
   private void assertRetainDeletes(KeepDeletedCells keepDeletedCells, Type[] types,
     MatchCode... expected) throws IOException {
+    assertRetainDeletes(keepDeletedCells, null, types, expected);
+  }
+
+  /**
+   * Build cells from the given types with decrementing timestamps. If qualifier is null,
+   * family-level types use empty qualifier and others use col1. If qualifier is specified, all
+   * types use that qualifier.
+   */
+  private void assertRetainDeletes(KeepDeletedCells keepDeletedCells, byte[] qualifier,
+    Type[] types, MatchCode... expected) throws IOException {
     long now = EnvironmentEdgeManager.currentTime();
     ScanInfo scanInfo = new ScanInfo(this.conf, fam1, 0, 1, ttl, keepDeletedCells,
       HConstants.DEFAULT_BLOCKSIZE, 0, rowComparator, false);
@@ -130,8 +165,13 @@ private void assertRetainDeletes(KeepDeletedCells keepDeletedCells, Type[] types
     long ts = now;
     List<MatchCode> actual = new ArrayList<>(expected.length);
     for (int i = 0; i < types.length; i++) {
-      boolean familyLevel = types[i] == Type.DeleteFamily || types[i] == Type.DeleteFamilyVersion;
-      byte[] qual = familyLevel ? HConstants.EMPTY_BYTE_ARRAY : col1;
+      byte[] qual;
+      if (qualifier != null) {
+        qual = qualifier;
+      } else {
+        boolean familyLevel = types[i] == Type.DeleteFamily || types[i] == Type.DeleteFamilyVersion;
+        qual = familyLevel ? HConstants.EMPTY_BYTE_ARRAY : col1;
+      }
       KeyValue kv = types[i] == Type.Put
         ? new KeyValue(row1, fam1, qual, ts, types[i], data)
         : new KeyValue(row1, fam1, qual, ts, types[i]);