This is an automated email from the ASF dual-hosted git repository. brfrn169 pushed a commit to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push: new 5104aa8 HBASE-24030 Add necessary validations to HRegion.checkAndMutate() and HRegion.checkAndRowMutate() (#1315) 5104aa8 is described below commit 5104aa80fa5ee5b7a7a75945b0c8f1db648a2912 Author: Toshihiro Suzuki <brfrn...@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 <vjas...@apache.org> Signed-off-by: Jan Hentschel <j...@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 c58a591..74b3a91 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 @@ -116,6 +116,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; @@ -4180,7 +4181,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); } @@ -4216,6 +4216,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(); @@ -4331,13 +4337,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 f58b82a..69da92d 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.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -2272,6 +2273,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 // ////////////////////////////////////////////////////////////////////////////