brfrn169 commented on a change in pull request #2630:
URL: https://github.com/apache/hbase/pull/2630#discussion_r530340359



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
##########
@@ -972,11 +1001,44 @@ private void doBatchOp(final RegionActionResult.Builder 
builder, final HRegion r
 
       OperationStatus[] codes = region.batchMutate(mArray, atomic, 
HConstants.NO_NONCE,
         HConstants.NO_NONCE);
+      if (atomic) {
+        List<ResultOrException> resultOrExceptions = new ArrayList<>();
+        List<Result> results = new ArrayList<>();
+        for (i = 0; i < codes.length; i++) {
+          if (codes[i].getResult() != null) {
+            results.add(codes[i].getResult());
+          }
+          if (i != 0) {
+            resultOrExceptions.add(getResultOrException(
+              ClientProtos.Result.getDefaultInstance(), i));
+          }
+        }
+
+        if (results.isEmpty()) {
+          resultOrExceptions.add(0, getResultOrException(
+            ClientProtos.Result.getDefaultInstance(), 0));
+        } else {
+          // Set the result of the Increment/Append operations to the first 
element of the
+          // ResultOrException list
+          List<Cell> cellList = new ArrayList<>();

Review comment:
       > Here we are in doBatchOp method, which will be called in 
RSRpcService.multi method, and the atomic flag is read from the protobuf 
message. This means the only place where we set atomic to true is for 
mutateRow? Maybe this is the case as for atomic batching we need to use the 
MultiRowMutationEndpoint, just asking.
   
   Yes, the only place where we set atomic to true is for mutateRow. In case of 
MultiRowMutationEndpoint, it calls Region.mutateRowsWithLocks() directly. So it 
doesn't call the doBatchOp method.
   
   > I suggest that if this is the case then we should add more comments in the 
code to explicitly say this so other developers will not be confusing.
   
   Sure I will add more comments. Thanks.




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


Reply via email to