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