[GitHub] [hbase] virajjasani commented on a change in pull request #1864: HBASE-24515 batch Increment/Append fails when retrying the RPC

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org