From 7f9622e0618e7e4c9f0c0b33c2c605cb9be1f527 Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Fri, 27 Mar 2026 18:24:22 +0900 Subject: [PATCH 1/2] HBASE-30036 Skip redundant delete markers during flush and minor compaction Add DeleteTracker.isRedundantDelete() to detect when a delete marker is already covered by a previously tracked delete of equal or broader scope. ScanDeleteTracker implements this for all four delete types: - DeleteFamily/DeleteFamilyVersion: covered by a tracked DeleteFamily - DeleteColumn/Delete: covered by a tracked DeleteFamily or DeleteColumn MinorCompactionScanQueryMatcher calls this check before including a delete marker, returning SEEK_NEXT_COL to skip past all remaining cells covered by the previously tracked delete. Compatible with KEEP_DELETED_CELLS. When set to TRUE, trackDelete() does not populate the delete tracker, so isRedundantDelete() always returns false and all markers are retained. --- .../querymatcher/DeleteTracker.java | 14 ++++ .../MinorCompactionScanQueryMatcher.java | 7 ++ .../querymatcher/ScanDeleteTracker.java | 22 ++++++ .../regionserver/TestStoreFileWriter.java | 11 ++- .../TestCompactionScanQueryMatcher.java | 76 +++++++++++++++++++ 5 files changed, 128 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/DeleteTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/DeleteTracker.java index 53816e9f3f34..1a66b99571aa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/DeleteTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/DeleteTracker.java @@ -83,6 +83,20 @@ enum DeleteResult { // deleted in strong semantics of versions(See MvccTracker) } + /** + * Check if the given delete marker is redundant, i.e., it is already covered by a previously + * tracked delete of equal or broader scope. A DeleteFamily is redundant if a DeleteFamily with a + * higher timestamp was already seen. A DeleteColumn is redundant if a DeleteColumn for the same + * qualifier with a higher timestamp, or a DeleteFamily with a higher timestamp, was already seen. + *

+ * This is a read-only check with no side effects on tracker state. + * @param cell the delete marker cell to check + * @return true if the delete marker is redundant and can be skipped + */ + default boolean isRedundantDelete(ExtendedCell cell) { + return false; + } + /** * Return the comparator passed to this delete tracker * @return the cell comparator diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java index 847eff44b318..ee04d0c005f2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java @@ -47,6 +47,13 @@ public MatchCode match(ExtendedCell cell) throws IOException { // we should not use this delete marker to mask any cell yet. return MatchCode.INCLUDE; } + // Check before tracking: an older DeleteColumn or DeleteFamily is redundant if a newer + // one of equal or broader scope was already seen. Must check before trackDelete() since + // 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)) { + return columns.getNextRowOrNextColumn(cell); + } trackDelete(cell); return MatchCode.INCLUDE; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanDeleteTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanDeleteTracker.java index efe09cce722f..65b6f2d21872 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanDeleteTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanDeleteTracker.java @@ -142,6 +142,28 @@ public DeleteResult isDeleted(ExtendedCell cell) { return DeleteResult.NOT_DELETED; } + @Override + public boolean isRedundantDelete(ExtendedCell cell) { + byte type = cell.getTypeByte(); + boolean coveredByFamily = hasFamilyStamp && cell.getTimestamp() <= familyStamp; + + if ( + type == KeyValue.Type.DeleteFamily.getCode() + || type == KeyValue.Type.DeleteFamilyVersion.getCode() + ) { + return coveredByFamily; + } + + boolean coveredByColumn = + deleteCell != null && deleteType == KeyValue.Type.DeleteColumn.getCode() + && CellUtil.matchingQualifier(cell, deleteCell) && cell.getTimestamp() <= deleteTimestamp; + + if (type == KeyValue.Type.DeleteColumn.getCode() || type == KeyValue.Type.Delete.getCode()) { + return coveredByFamily || coveredByColumn; + } + return false; + } + @Override public boolean isEmpty() { return deleteCell == null && !hasFamilyStamp && familyVersionStamps.isEmpty(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileWriter.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileWriter.java index 6c27bc4082b0..de84d4daa5e9 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileWriter.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFileWriter.java @@ -177,8 +177,15 @@ public void testCompactedFiles() throws Exception { stores[0].getStorefilesCount()); regions[1].compact(false); - assertEquals(flushCount - stores[1].getCompactedFiles().size() + 2, - stores[1].getStorefilesCount()); + // HBASE-30036 skips redundant delete markers during minor compaction, so the historical + // file may end up empty and not be created. The count can be +1 or +2. + int minorCompactedCount = stores[1].getStorefilesCount(); + int expectedMin = flushCount - stores[1].getCompactedFiles().size() + 1; + int expectedMax = flushCount - stores[1].getCompactedFiles().size() + 2; + assertTrue( + "Expected store file count between " + expectedMin + " and " + expectedMax + " but was " + + minorCompactedCount, + minorCompactedCount >= expectedMin && minorCompactedCount <= expectedMax); verifyCells(); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java index 400c2fd2b60b..d3a8b1b299f5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.regionserver.querymatcher; import static org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher.MatchCode.INCLUDE; +import static org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher.MatchCode.SEEK_NEXT_COL; import static org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher.MatchCode.SKIP; import static org.junit.Assert.assertEquals; @@ -75,6 +76,81 @@ public void testMatch_PartialRangeDropDeletes() throws Exception { testDropDeletes(row2, row3, new byte[][] { row1, row1 }, INCLUDE, INCLUDE); } + /** + * Test redundant delete marker handling with COMPACT_RETAIN_DELETES. Cells are auto-generated + * from the given types with decrementing timestamps. + */ + @Test + public void testSkipsRedundantDeleteMarkers() throws IOException { + // Interleaved DeleteColumn + Put. First DC included, put triggers SEEK_NEXT_COL. + assertRetainDeletes(new Type[] { Type.DeleteColumn, Type.Put, Type.DeleteColumn }, INCLUDE, + SEEK_NEXT_COL); + + // Contiguous DeleteColumn. First included, rest redundant. + assertRetainDeletes(new Type[] { Type.DeleteColumn, Type.DeleteColumn, Type.DeleteColumn }, + INCLUDE, SEEK_NEXT_COL, SEEK_NEXT_COL); + + // Contiguous DeleteFamily. First included, rest redundant. + 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. + assertRetainDeletes(new Type[] { Type.DeleteFamily, Type.DeleteFamilyVersion, Type.DeleteFamily, + Type.DeleteFamilyVersion }, INCLUDE, SEEK_NEXT_COL, SEEK_NEXT_COL, SEEK_NEXT_COL); + + // Delete (version) covered by DeleteColumn. + assertRetainDeletes(new Type[] { Type.DeleteColumn, Type.Delete, Type.Delete, Type.Delete }, + INCLUDE, SEEK_NEXT_COL, SEEK_NEXT_COL, SEEK_NEXT_COL); + + // KEEP_DELETED_CELLS=TRUE: all markers retained. + assertRetainDeletes(KeepDeletedCells.TRUE, + new Type[] { Type.DeleteColumn, Type.DeleteColumn, Type.DeleteColumn }, INCLUDE, INCLUDE, + INCLUDE); + } + + private void assertRetainDeletes(Type[] types, MatchCode... expected) throws IOException { + assertRetainDeletes(KeepDeletedCells.FALSE, 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 { + long now = EnvironmentEdgeManager.currentTime(); + ScanInfo scanInfo = new ScanInfo(this.conf, fam1, 0, 1, ttl, keepDeletedCells, + HConstants.DEFAULT_BLOCKSIZE, 0, rowComparator, false); + CompactionScanQueryMatcher qm = CompactionScanQueryMatcher.create(scanInfo, + ScanType.COMPACT_RETAIN_DELETES, 0L, PrivateConstants.OLDEST_TIMESTAMP, + PrivateConstants.OLDEST_TIMESTAMP, now, null, null, null); + qm.setToNewRow(KeyValueUtil.createFirstOnRow(row1)); + + long ts = now; + List 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; + KeyValue kv = types[i] == Type.Put + ? new KeyValue(row1, fam1, qual, ts, types[i], data) + : new KeyValue(row1, fam1, qual, ts, types[i]); + actual.add(qm.match(kv)); + if (actual.size() >= expected.length) { + break; + } + // Decrement ts for next cell, but keep same ts when the next type has lower type code + // at the same logical position (e.g. DF then DFV at the same timestamp). + if (i + 1 < types.length && types[i + 1].getCode() < types[i].getCode()) { + continue; + } + ts--; + } + for (int i = 0; i < expected.length; i++) { + assertEquals("Mismatch at index " + i, expected[i], actual.get(i)); + } + } + private void testDropDeletes(byte[] from, byte[] to, byte[][] rows, MatchCode... expected) throws IOException { long now = EnvironmentEdgeManager.currentTime(); From 78effd5d8d14ee14e4b630c0c2aefd7b9463cb5e Mon Sep 17 00:00:00 2001 From: Junegunn Choi Date: Wed, 1 Apr 2026 09:03:55 +0900 Subject: [PATCH 2/2] HBASE-30036 Fix redundant DeleteColumn with empty qualifier skipping DeleteFamily --- .../MinorCompactionScanQueryMatcher.java | 7 +++ .../TestCompactionScanQueryMatcher.java | 48 +++++++++++++++++-- 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java index ee04d0c005f2..6fbb138197ea 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MinorCompactionScanQueryMatcher.java +++ b/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); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java index d3a8b1b299f5..17681d2f0149 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestCompactionScanQueryMatcher.java +++ b/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,10 +109,34 @@ 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, @@ -119,6 +144,16 @@ private void assertRetainDeletes(Type[] types, MatchCode... expected) throws IOE */ 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 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]);