Apache9 commented on a change in pull request #2630:
URL: https://github.com/apache/hbase/pull/2630#discussion_r530057114
##########
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:
Seems my comment is disappeared, maybe I forgot to push the comment
button. The concern here is how we can make sure that when arriving this
branch, we are going to treat the result for mutateRow. 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. And 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.
----------------------------------------------------------------
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]
