From 81cb29801403dd9ae7e94b6f721260cadd6e6950 Mon Sep 17 00:00:00 2001 From: ChiaPing Tsai Date: Tue, 7 Mar 2017 01:20:34 +0800 Subject: [PATCH] HBASE-17734 Guard against possibly copying the qualifier in the ScanDeleteTracker Signed-off-by: tedyu --- .../CompactionScanQueryMatcher.java | 6 +++ .../querymatcher/DeleteTracker.java | 3 +- .../querymatcher/LegacyScanQueryMatcher.java | 6 +++ .../NormalUserScanQueryMatcher.java | 6 +++ .../querymatcher/ScanDeleteTracker.java | 37 +++++++++++-------- .../VisibilityScanDeleteTracker.java | 18 ++++----- 6 files changed, 51 insertions(+), 25 deletions(-) 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 95df581c71f..b3c14d77e7d 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 @@ -55,6 +55,12 @@ public abstract class CompactionScanQueryMatcher extends ScanQueryMatcher { this.keepDeletedCells = scanInfo.getKeepDeletedCells(); } + @Override + public void beforeShipped() throws IOException { + super.beforeShipped(); + deletes.beforeShipped(); + } + @Override public boolean hasNullColumnInQuery() { return true; 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 4e1ba4ec2e1..45b170e8cf5 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 @@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.regionserver.querymatcher; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.regionserver.ShipperListener; /** * This interface is used for the tracking and enforcement of Deletes during the course of a Get or @@ -32,7 +33,7 @@ import org.apache.hadoop.hbase.Cell; * */ @InterfaceAudience.Private -public interface DeleteTracker { +public interface DeleteTracker extends ShipperListener { /** * Add the specified cell to the list of deletes to check against for this row operation. 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 4ba685f13c7..94440ccccc3 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 @@ -147,6 +147,12 @@ public class LegacyScanQueryMatcher extends ScanQueryMatcher { this.dropDeletesToRow = Preconditions.checkNotNull(dropDeletesToRow); } + @Override + public void beforeShipped() throws IOException { + super.beforeShipped(); + deletes.beforeShipped(); + } + @Override public MatchCode match(Cell cell) throws IOException { if (filter != null && filter.filterAllRemaining()) { 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 8f5059fe4f0..d5fda546c58 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 @@ -50,6 +50,12 @@ public abstract class NormalUserScanQueryMatcher extends UserScanQueryMatcher { this.seePastDeleteMarkers = scanInfo.getKeepDeletedCells() != KeepDeletedCells.FALSE; } + @Override + public void beforeShipped() throws IOException { + super.beforeShipped(); + deletes.beforeShipped(); + } + @Override public MatchCode match(Cell cell) throws IOException { if (filter != null && filter.filterAllRemaining()) { 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 450a30e4000..eb6e50333cb 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 @@ -18,6 +18,7 @@ package org.apache.hadoop.hbase.regionserver.querymatcher; +import java.io.IOException; import java.util.SortedSet; import java.util.TreeSet; @@ -26,6 +27,7 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellUtil; import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.util.Bytes; /** @@ -48,9 +50,7 @@ public class ScanDeleteTracker implements DeleteTracker { protected boolean hasFamilyStamp = false; protected long familyStamp = 0L; protected SortedSet familyVersionStamps = new TreeSet(); - protected byte[] deleteBuffer = null; - protected int deleteOffset = 0; - protected int deleteLength = 0; + protected Cell deleteCell = null; protected byte deleteType = 0; protected long deleteTimestamp = 0L; @@ -74,16 +74,14 @@ public class ScanDeleteTracker implements DeleteTracker { return; } - if (deleteBuffer != null && type < deleteType) { + if (deleteCell != null && type < deleteType) { // same column, so ignore less specific delete - if (CellUtil.matchingQualifier(cell, deleteBuffer, deleteOffset, deleteLength)) { + if (CellUtil.matchingQualifier(cell, deleteCell)) { return; } } // new column, or more general delete type - deleteBuffer = cell.getQualifierArray(); - deleteOffset = cell.getQualifierOffset(); - deleteLength = cell.getQualifierLength(); + deleteCell = cell; deleteType = type; deleteTimestamp = timestamp; } @@ -106,8 +104,8 @@ public class ScanDeleteTracker implements DeleteTracker { return DeleteResult.FAMILY_VERSION_DELETED; } - if (deleteBuffer != null) { - int ret = -(CellComparator.compareQualifiers(cell, deleteBuffer, deleteOffset, deleteLength)); + if (deleteCell != null) { + int ret = -(CellComparator.compareQualifiers(cell, deleteCell)); if (ret == 0) { if (deleteType == KeyValue.Type.DeleteColumn.getCode()) { return DeleteResult.COLUMN_DELETED; @@ -121,13 +119,15 @@ public class ScanDeleteTracker implements DeleteTracker { assert timestamp < deleteTimestamp; // different timestamp, let's clear the buffer. - deleteBuffer = null; + deleteCell = null; } else if (ret < 0) { // Next column case. - deleteBuffer = null; + deleteCell = null; } else { throw new IllegalStateException("isDelete failed: deleteBuffer=" - + Bytes.toStringBinary(deleteBuffer, deleteOffset, deleteLength) + ", qualifier=" + + Bytes.toStringBinary(deleteCell.getQualifierArray(), + deleteCell.getQualifierOffset(), deleteCell.getQualifierLength()) + + ", qualifier=" + Bytes.toStringBinary(cell.getQualifierArray(), cell.getQualifierOffset(), cell.getQualifierLength()) + ", timestamp=" + timestamp + ", comparison result: " + ret); @@ -139,7 +139,7 @@ public class ScanDeleteTracker implements DeleteTracker { @Override public boolean isEmpty() { - return deleteBuffer == null && !hasFamilyStamp && familyVersionStamps.isEmpty(); + return deleteCell == null && !hasFamilyStamp && familyVersionStamps.isEmpty(); } @Override @@ -148,7 +148,7 @@ public class ScanDeleteTracker implements DeleteTracker { hasFamilyStamp = false; familyStamp = 0L; familyVersionStamps.clear(); - deleteBuffer = null; + deleteCell = null; } @Override @@ -156,4 +156,11 @@ public class ScanDeleteTracker implements DeleteTracker { public void update() { this.reset(); } + + @Override + public void beforeShipped() throws IOException { + if (deleteCell != null) { + deleteCell = KeyValueUtil.toNewKeyCell(deleteCell); + } + } } 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 4e27bbfa7c2..2595fe054bf 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 @@ -87,8 +87,8 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { return; } // new column, or more general delete type - if (deleteBuffer != null) { - if (!(CellUtil.matchingQualifier(delCell, deleteBuffer, deleteOffset, deleteLength))) { + if (deleteCell != null) { + if (!(CellUtil.matchingQualifier(delCell, deleteCell))) { // A case where there are deletes for a column qualifier but there are // no corresponding puts for them. Rare case. visibilityTagsDeleteColumns = null; @@ -104,9 +104,7 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { visiblityTagsDeleteColumnVersion = null; } } - deleteBuffer = delCell.getQualifierArray(); - deleteOffset = delCell.getQualifierOffset(); - deleteLength = delCell.getQualifierLength(); + deleteCell = delCell; deleteType = type; deleteTimestamp = timestamp; extractDeleteCellVisTags(delCell, KeyValue.Type.codeToType(type)); @@ -243,8 +241,8 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } } } - if (deleteBuffer != null) { - int ret = CellComparator.compareQualifiers(cell, deleteBuffer, deleteOffset, deleteLength); + if (deleteCell != null) { + int ret = CellComparator.compareQualifiers(cell, deleteCell); if (ret == 0) { if (deleteType == KeyValue.Type.DeleteColumn.getCode()) { if (visibilityTagsDeleteColumns != null) { @@ -304,13 +302,15 @@ public class VisibilityScanDeleteTracker extends ScanDeleteTracker { } } else if (ret > 0) { // Next column case. - deleteBuffer = null; + deleteCell = null; // Can nullify this because we are moving to the next column visibilityTagsDeleteColumns = null; visiblityTagsDeleteColumnVersion = null; } else { throw new IllegalStateException("isDeleted failed: deleteBuffer=" - + Bytes.toStringBinary(deleteBuffer, deleteOffset, deleteLength) + ", qualifier=" + + Bytes.toStringBinary(deleteCell.getQualifierArray(), + deleteCell.getQualifierOffset(), deleteCell.getQualifierLength()) + + ", qualifier=" + Bytes.toStringBinary(cell.getQualifierArray(), cell.getQualifierOffset(), cell.getQualifierLength()) + ", timestamp=" + timestamp + ", comparison result: " + ret);