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 faa3a068e98..d13cc62a2b5 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 3720773a633..423208e7c8f 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 // ////////////////////////////////////////////////////////////////////////////