virajjasani commented on a change in pull request #1864:
URL: https://github.com/apache/hbase/pull/1864#discussion_r436339955



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -794,6 +792,29 @@ private Result increment(final HRegion region, final 
OperationQuota quota,
     return r == null ? Result.EMPTY_RESULT : r;
   }
 
+  private Get toGet(final Mutation mutation) throws IOException {
+    assert mutation instanceof Increment || mutation instanceof Append;

Review comment:
       Instead of `assert` statement, we can throw AssertionError with if 
condition? Just that assert might not be enabled server side.

##########
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestIncrementsFromClientSide.java
##########
@@ -131,6 +132,44 @@ public void testDuplicateIncrement() throws Exception {
     }
   }
 
+  /**
+   * Test batch increment result when there are duplicate rpc request.
+   */
+  @Test
+  public void testDuplicateBatchIncrement() throws Exception {
+    HTableDescriptor hdt = 
TEST_UTIL.createTableDescriptor(TableName.valueOf(name.getMethodName()));

Review comment:
       Same here

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -794,6 +792,29 @@ private Result increment(final HRegion region, final 
OperationQuota quota,
     return r == null ? Result.EMPTY_RESULT : r;
   }
 
+  private Get toGet(final Mutation mutation) throws IOException {

Review comment:
       Looks like a candidate for `static` util method. Maybe we can move this 
to some Mutation related util class? This file anyways has lot of code (with 
more APIs, it will just increase :) )

##########
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestFromClientSide.java
##########
@@ -147,6 +148,47 @@ public void testDuplicateAppend() throws Exception {
     }
   }
 
+  /**
+   * Test batch append result when there are duplicate rpc request.
+   */
+  @Test
+  public void testDuplicateBatchAppend() throws Exception {
+    HTableDescriptor hdt = TEST_UTIL
+      .createTableDescriptor(name.getTableName(), 
HColumnDescriptor.DEFAULT_MIN_VERSIONS, 3,

Review comment:
       Use relevant method for createModifyableTableDescriptor? Given that 
HTableDescriptor is deprecated.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to