You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by br...@apache.org on 2020/03/22 02:56:07 UTC
[hbase] branch master updated: HBASE-24030 Add necessary
validations to HRegion.checkAndMutate() and HRegion.checkAndRowMutate()
(#1315)
This is an automated email from the ASF dual-hosted git repository.
brfrn169 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 9b4e75a HBASE-24030 Add necessary validations to HRegion.checkAndMutate() and HRegion.checkAndRowMutate() (#1315)
9b4e75a is described below
commit 9b4e75a7f6b876e00c6360dd69adb13d3200b1f2
Author: Toshihiro Suzuki <br...@gmail.com>
AuthorDate: Sun Mar 22 11:55:52 2020 +0900
HBASE-24030 Add necessary validations to HRegion.checkAndMutate() and HRegion.checkAndRowMutate() (#1315)
Signed-off-by: Viraj Jasani <vj...@apache.org>
Signed-off-by: Jan Hentschel <ja...@apache.org>
---
.../apache/hadoop/hbase/regionserver/HRegion.java | 16 ++++-
.../hadoop/hbase/regionserver/TestHRegion.java | 73 ++++++++++++++++++++++
2 files changed, 86 insertions(+), 3 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
index faa3a06..d13cc62 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
@@ -115,6 +115,7 @@ import org.apache.hadoop.hbase.client.RegionInfo;
import org.apache.hadoop.hbase.client.RegionInfoBuilder;
import org.apache.hadoop.hbase.client.RegionReplicaUtil;
import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Row;
import org.apache.hadoop.hbase.client.RowMutations;
import org.apache.hadoop.hbase.client.Scan;
import org.apache.hadoop.hbase.client.TableDescriptor;
@@ -4207,7 +4208,6 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
@Override
public boolean checkAndMutate(byte[] row, byte[] family, byte[] qualifier, CompareOperator op,
ByteArrayComparable comparator, TimeRange timeRange, Mutation mutation) throws IOException {
- checkMutationType(mutation, row);
return doCheckAndRowMutate(row, family, qualifier, op, comparator, null, timeRange, null,
mutation);
}
@@ -4243,6 +4243,12 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
// need these commented out checks.
// if (rowMutations == null && mutation == null) throw new DoNotRetryIOException("Both null");
// if (rowMutations != null && mutation != null) throw new DoNotRetryIOException("Both set");
+ if (mutation != null) {
+ checkMutationType(mutation);
+ checkRow(mutation, row);
+ } else {
+ checkRow(rowMutations, row);
+ }
checkReadOnly();
// TODO, add check for value length also move this check to the client
checkResources();
@@ -4358,13 +4364,17 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver, Regi
}
}
- private void checkMutationType(final Mutation mutation, final byte [] row)
+ private void checkMutationType(final Mutation mutation)
throws DoNotRetryIOException {
boolean isPut = mutation instanceof Put;
if (!isPut && !(mutation instanceof Delete)) {
throw new org.apache.hadoop.hbase.DoNotRetryIOException("Action must be Put or Delete");
}
- if (!Bytes.equals(row, mutation.getRow())) {
+ }
+
+ private void checkRow(final Row action, final byte[] row)
+ throws DoNotRetryIOException {
+ if (!Bytes.equals(row, action.getRow())) {
throw new org.apache.hadoop.hbase.DoNotRetryIOException("Action's getRow must match");
}
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
index 3720773..423208e 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java
@@ -74,6 +74,7 @@ import org.apache.hadoop.hbase.CellBuilderType;
import org.apache.hadoop.hbase.CellUtil;
import org.apache.hadoop.hbase.CompareOperator;
import org.apache.hadoop.hbase.CompatibilitySingletonFactory;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
import org.apache.hadoop.hbase.DroppedSnapshotException;
import org.apache.hadoop.hbase.ExtendedCellBuilderFactory;
import org.apache.hadoop.hbase.HBaseClassTestRule;
@@ -2268,6 +2269,78 @@ public class TestHRegion {
assertTrue(region.get(new Get(row).addColumn(FAMILY, Bytes.toBytes("A"))).isEmpty());
}
+ @Test
+ public void testCheckAndMutate_wrongMutationType() throws Throwable {
+ // Setting up region
+ this.region = initHRegion(tableName, method, CONF, fam1);
+
+ try {
+ region.checkAndMutate(row, fam1, qual1, CompareOperator.EQUAL, new BinaryComparator(value1),
+ new Increment(row).addColumn(fam1, qual1, 1));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action must be Put or Delete", e.getMessage());
+ }
+
+ try {
+ region.checkAndMutate(row,
+ new SingleColumnValueFilter(fam1, qual1, CompareOperator.EQUAL, value1),
+ new Increment(row).addColumn(fam1, qual1, 1));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action must be Put or Delete", e.getMessage());
+ }
+ }
+
+ @Test
+ public void testCheckAndMutate_wrongRow() throws Throwable {
+ final byte[] wrongRow = Bytes.toBytes("wrongRow");
+
+ // Setting up region
+ this.region = initHRegion(tableName, method, CONF, fam1);
+
+ try {
+ region.checkAndMutate(row, fam1, qual1, CompareOperator.EQUAL, new BinaryComparator(value1),
+ new Put(wrongRow).addColumn(fam1, qual1, value1));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action's getRow must match", e.getMessage());
+ }
+
+ try {
+ region.checkAndMutate(row,
+ new SingleColumnValueFilter(fam1, qual1, CompareOperator.EQUAL, value1),
+ new Put(wrongRow).addColumn(fam1, qual1, value1));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action's getRow must match", e.getMessage());
+ }
+
+ try {
+ region.checkAndRowMutate(row, fam1, qual1, CompareOperator.EQUAL,
+ new BinaryComparator(value1),
+ new RowMutations(wrongRow)
+ .add((Mutation) new Put(wrongRow)
+ .addColumn(fam1, qual1, value1))
+ .add((Mutation) new Delete(wrongRow).addColumns(fam1, qual2)));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action's getRow must match", e.getMessage());
+ }
+
+ try {
+ region.checkAndRowMutate(row,
+ new SingleColumnValueFilter(fam1, qual1, CompareOperator.EQUAL, value1),
+ new RowMutations(wrongRow)
+ .add((Mutation) new Put(wrongRow)
+ .addColumn(fam1, qual1, value1))
+ .add((Mutation) new Delete(wrongRow).addColumns(fam1, qual2)));
+ fail("should throw DoNotRetryIOException");
+ } catch (DoNotRetryIOException e) {
+ assertEquals("Action's getRow must match", e.getMessage());
+ }
+ }
+
// ////////////////////////////////////////////////////////////////////////////
// Delete tests
// ////////////////////////////////////////////////////////////////////////////