diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/CompactionScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/CompactionScanQueryMatcher.java index d3224dc..c55850a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/CompactionScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/CompactionScanQueryMatcher.java @@ -93,8 +93,8 @@ public abstract class CompactionScanQueryMatcher extends ScanQueryMatcher { // minVerions is larger than 0(otherwise we will just return at preCheck). So here we still // need to track the delete marker to see if it masks some cells. if (keepDeletedCells == KeepDeletedCells.FALSE - || (keepDeletedCells == KeepDeletedCells.TTL && cell.getTimestamp() < oldestUnexpiredTS)) { - deletes.add(cell); + || (keepDeletedCells == KeepDeletedCells.TTL && timestamp < oldestUnexpiredTS)) { + deletes.add(cell, timestamp, typeByte); } } 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 4e1ba4e..76ab495 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 @@ -39,15 +39,18 @@ public interface DeleteTracker { *
* This is called when a Delete is encountered in a StoreFile. * @param cell - the delete cell + * @param timestamp - the timestamp of the cell + * @param type - the type of the cell */ - void add(Cell cell); + void add(Cell cell, long timestamp, byte type); /** * Check if the specified cell buffer has been deleted by a previously seen delete. * @param cell - current cell to check if deleted by a previously seen delete + * @param timestamp - the timestamp of the cell * @return deleteResult The result tells whether the KeyValue is deleted and why */ - DeleteResult isDeleted(Cell cell); + DeleteResult isDeleted(Cell cell, long timestamp); /** * @return true if there are no current delete, false otherwise diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/LegacyScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/LegacyScanQueryMatcher.java index ea4bd97..cdfd0e7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/LegacyScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/LegacyScanQueryMatcher.java @@ -168,10 +168,9 @@ public class LegacyScanQueryMatcher extends ScanQueryMatcher { * 7. Delete marker need to be version counted together with puts * they affect */ - long timestamp = cell.getTimestamp(); - byte typeByte = cell.getTypeByte(); + typeByte = cell.getTypeByte(); long mvccVersion = cell.getSequenceId(); - if (CellUtil.isDelete(cell)) { + if (CellUtil.isDelete(typeByte)) { if (keepDeletedCells == KeepDeletedCells.FALSE || (keepDeletedCells == KeepDeletedCells.TTL && timestamp < oldestUnexpiredTS)) { // first ignore delete markers if the scanner can do so, and the @@ -182,7 +181,7 @@ public class LegacyScanQueryMatcher extends ScanQueryMatcher { // rows that could still be seen by a scanner from being collected boolean includeDeleteMarker = tr.withinOrAfterTimeRange(timestamp); if (includeDeleteMarker && mvccVersion <= maxReadPointToTrackVersions) { - this.deletes.add(cell); + this.deletes.add(cell, timestamp, typeByte); } // Can't early out now, because DelFam come before any other keys } @@ -211,7 +210,7 @@ public class LegacyScanQueryMatcher extends ScanQueryMatcher { // note the following next else if... // delete marker are not subject to other delete markers } else if (!this.deletes.isEmpty()) { - DeleteResult deleteResult = deletes.isDeleted(cell); + DeleteResult deleteResult = deletes.isDeleted(cell, timestamp); switch (deleteResult) { case FAMILY_DELETED: case COLUMN_DELETED: diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MajorCompactionScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MajorCompactionScanQueryMatcher.java index 6a2ed40..93a250c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MajorCompactionScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/MajorCompactionScanQueryMatcher.java @@ -41,7 +41,6 @@ public class MajorCompactionScanQueryMatcher extends DropDeletesCompactionScanQu if (returnCode != null) { return returnCode; } - long timestamp = cell.getTimestamp(); long mvccVersion = cell.getSequenceId(); // The delete logic is pretty complicated now. @@ -56,7 +55,8 @@ public class MajorCompactionScanQueryMatcher extends DropDeletesCompactionScanQu // 7. Delete marker need to be version counted together with puts // they affect // - if (CellUtil.isDelete(cell)) { + typeByte = cell.getTypeByte(); + if (CellUtil.isDelete(typeByte)) { if (mvccVersion > maxReadPointToTrackVersions) { // We can not drop this delete marker yet, and also we should not use this delete marker to // mask any cell yet. @@ -74,7 +74,7 @@ public class MajorCompactionScanQueryMatcher extends DropDeletesCompactionScanQu } } // Skip checking column since we do not remove column during compaction. - return columns.checkVersions(cell, timestamp, cell.getTypeByte(), + return columns.checkVersions(cell, timestamp, typeByte, mvccVersion > maxReadPointToTrackVersions); } } 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 3b6acde..15956a2 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 @@ -42,7 +42,8 @@ public class MinorCompactionScanQueryMatcher extends CompactionScanQueryMatcher return returnCode; } long mvccVersion = cell.getSequenceId(); - if (CellUtil.isDelete(cell)) { + typeByte = cell.getTypeByte(); + if (CellUtil.isDelete(typeByte)) { if (mvccVersion > maxReadPointToTrackVersions) { // we should not use this delete marker to mask any cell yet. return MatchCode.INCLUDE; @@ -55,7 +56,7 @@ public class MinorCompactionScanQueryMatcher extends CompactionScanQueryMatcher return returnCode; } // Skip checking column since we do not remove column during compaction. - return columns.checkVersions(cell, cell.getTimestamp(), cell.getTypeByte(), + return columns.checkVersions(cell, timestamp, typeByte, mvccVersion > maxReadPointToTrackVersions); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NormalUserScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NormalUserScanQueryMatcher.java index 3942f04..d3e4fe6 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NormalUserScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/NormalUserScanQueryMatcher.java @@ -59,12 +59,12 @@ public class NormalUserScanQueryMatcher extends UserScanQueryMatcher { if (returnCode != null) { return returnCode; } - long timestamp = cell.getTimestamp(); - if (CellUtil.isDelete(cell)) { + typeByte = cell.getTypeByte(); + if (CellUtil.isDelete(typeByte)) { boolean includeDeleteMarker = seePastDeleteMarkers ? tr.withinTimeRange(timestamp) : tr.withinOrAfterTimeRange(timestamp); if (includeDeleteMarker) { - this.deletes.add(cell); + this.deletes.add(cell, timestamp, typeByte); } return MatchCode.SKIP; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/RawScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/RawScanQueryMatcher.java index acdae90..80073d0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/RawScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/RawScanQueryMatcher.java @@ -44,6 +44,7 @@ public class RawScanQueryMatcher extends UserScanQueryMatcher { if (returnCode != null) { return returnCode; } + typeByte = cell.getTypeByte(); // For a raw scan, we do not filter out any cells by delete marker, and delete marker is also // returned, so we do not need to track delete. return matchColumn(cell); 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 450a30e..96ca326 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 @@ -61,9 +61,7 @@ public class ScanDeleteTracker implements DeleteTracker { * @param cell - the delete cell */ @Override - public void add(Cell cell) { - long timestamp = cell.getTimestamp(); - byte type = cell.getTypeByte(); + public void add(Cell cell, long timestamp, byte type) { if (!hasFamilyStamp || timestamp > familyStamp) { if (type == KeyValue.Type.DeleteFamily.getCode()) { hasFamilyStamp = true; @@ -96,8 +94,7 @@ public class ScanDeleteTracker implements DeleteTracker { * @return deleteResult */ @Override - public DeleteResult isDeleted(Cell cell) { - long timestamp = cell.getTimestamp(); + public DeleteResult isDeleted(Cell cell, long timestamp) { if (hasFamilyStamp && timestamp <= familyStamp) { return DeleteResult.FAMILY_DELETED; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java index 82aae6c..2385f44 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/ScanQueryMatcher.java @@ -32,7 +32,6 @@ import org.apache.hadoop.hbase.TagType; import org.apache.hadoop.hbase.TagUtil; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.filter.Filter; -import org.apache.hadoop.hbase.regionserver.KeyValueScanner; import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; import org.apache.hadoop.hbase.regionserver.ScanInfo; import org.apache.hadoop.hbase.regionserver.ShipperListener; @@ -127,6 +126,12 @@ public abstract class ScanQueryMatcher implements ShipperListener { protected boolean stickyNextRow; + // Keep current cell's timestamp for reuse + protected long timestamp; + + // Keep current cell's type for reuse + protected byte typeByte; + protected ScanQueryMatcher(byte[] startRow, ScanInfo scanInfo, ColumnTracker columns, long oldestUnexpiredTS, long now) { this.rowComparator = scanInfo.getComparator(); @@ -190,7 +195,7 @@ public abstract class ScanQueryMatcher implements ShipperListener { return MatchCode.SEEK_NEXT_ROW; } - long timestamp = cell.getTimestamp(); + timestamp = cell.getTimestamp(); // check if this is a fake cell. The fake cell is an optimization, we should make the scanner // seek to next column or next row. See StoreFileScanner.requestSeek for more details. // check for early out based on timestamp alone @@ -208,7 +213,7 @@ public abstract class ScanQueryMatcher implements ShipperListener { if (deletes.isEmpty()) { return null; } - DeleteResult deleteResult = deletes.isDeleted(cell); + DeleteResult deleteResult = deletes.isDeleted(cell, timestamp); switch (deleteResult) { case FAMILY_DELETED: case COLUMN_DELETED: diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/StripeCompactionScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/StripeCompactionScanQueryMatcher.java index c1e63b4..81b11e2 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/StripeCompactionScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/StripeCompactionScanQueryMatcher.java @@ -55,7 +55,8 @@ public class StripeCompactionScanQueryMatcher extends DropDeletesCompactionScanQ return returnCode; } long mvccVersion = cell.getSequenceId(); - if (CellUtil.isDelete(cell)) { + typeByte = cell.getTypeByte(); + if (CellUtil.isDelete(typeByte)) { if (mvccVersion > maxReadPointToTrackVersions) { return MatchCode.INCLUDE; } @@ -77,7 +78,7 @@ public class StripeCompactionScanQueryMatcher extends DropDeletesCompactionScanQ } } // Skip checking column since we do not remove column during compaction. - return columns.checkVersions(cell, cell.getTimestamp(), cell.getTypeByte(), + return columns.checkVersions(cell, timestamp, typeByte, mvccVersion > maxReadPointToTrackVersions); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/UserScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/UserScanQueryMatcher.java index ec7fc11..e0d3be0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/UserScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/querymatcher/UserScanQueryMatcher.java @@ -89,7 +89,6 @@ public abstract class UserScanQueryMatcher extends ScanQueryMatcher { } protected final MatchCode matchColumn(Cell cell) throws IOException { - long timestamp = cell.getTimestamp(); int tsCmp = tr.compare(timestamp); if (tsCmp > 0) { return MatchCode.SKIP; @@ -97,7 +96,6 @@ public abstract class UserScanQueryMatcher extends ScanQueryMatcher { if (tsCmp < 0) { return columns.getNextRowOrNextColumn(cell); } - byte typeByte = cell.getTypeByte(); // STEP 1: Check if the column is part of the requested columns MatchCode colChecker = columns.checkColumn(cell, typeByte); if (colChecker != MatchCode.INCLUDE) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java index 4e27bbf..c20c362 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/visibility/VisibilityScanDeleteTracker.java @@ -70,10 +70,8 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } @Override - public void add(Cell delCell) { + public void add(Cell delCell, long timestamp, byte type) { //Cannot call super.add because need to find if the delete needs to be considered - long timestamp = delCell.getTimestamp(); - byte type = delCell.getTypeByte(); if (type == KeyValue.Type.DeleteFamily.getCode()) { hasFamilyStamp = true; boolean hasVisTag = extractDeleteCellVisTags(delCell, KeyValue.Type.DeleteFamily); @@ -174,8 +172,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } @Override - public DeleteResult isDeleted(Cell cell) { - long timestamp = cell.getTimestamp(); + public DeleteResult isDeleted(Cell cell, long timestamp) { try { if (hasFamilyStamp) { if (visibilityTagsDeleteFamily != null) { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestScanDeleteTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestScanDeleteTracker.java index fce35bd..2541de5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestScanDeleteTracker.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/querymatcher/TestScanDeleteTracker.java @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.regionserver.querymatcher; import static org.junit.Assert.*; +import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.regionserver.querymatcher.DeleteTracker.DeleteResult; import org.apache.hadoop.hbase.testclassification.RegionServerTests; @@ -33,13 +34,25 @@ import org.junit.experimental.categories.Category; @Category({ RegionServerTests.class, SmallTests.class }) public class TestScanDeleteTracker { - private ScanDeleteTracker sdt; + private ExtendScanDeleteTracker sdt; private long timestamp = 10L; + public static class ExtendScanDeleteTracker extends ScanDeleteTracker { + + public void add(Cell cell) { + add(cell, cell.getTimestamp(), cell.getTypeByte()); + } + + public DeleteResult isDeleted(Cell cell) { + return isDeleted(cell, cell.getTimestamp()); + } + + } + @Before public void setUp() throws Exception { - sdt = new ScanDeleteTracker(); + sdt = new ExtendScanDeleteTracker(); } @Test