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



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1957,17 +1959,27 @@ public boolean normalizeRegions() throws IOException {
             continue;
           }
 
-          // as of this writing, `plan.execute()` is non-blocking, so there's 
no artificial rate-
-          // limiting of merge requests due to this serial loop.
+          // as of this writing, `plan.submit()` is non-blocking and uses 
Async Admin APIs to
+          // submit task , so there's no artificial rate-
+          // limiting of merge/split requests due to this serial loop.
           for (NormalizationPlan plan : plans) {
-            plan.execute(admin);
+            Future<Void> future = plan.submit(admin);
+            submittedPlanList.add(future);
             if (plan.getType() == PlanType.SPLIT) {
               splitPlanCount++;
             } else if (plan.getType() == PlanType.MERGE) {
               mergePlanCount++;
             }
           }
         }
+        for (Future<?> submittedPlan : submittedPlanList) {
+          try {
+            submittedPlan.get();
+          } catch (Exception e) {
+            normalizationPlanFailureCount++;

Review comment:
       Although going through client interface is not necessary, in some way it 
might simplify things: 
   
   1. We won't have to re-write the code to convert encodedRegionName to 
RegionInfo, derive TableName etc and then use `MasterServices` interface, which 
requires additional info.
   2. Since our goal is to `submit` async plans and not `execute` blocking 
operations, Admin interface already has nice non-blocking utility, which is 
again something we will have to implement on our own if we directly want to use 
`MasterServices` (which does use ProcV2 but we want to finally log: x/y plans 
succeeded, and for that to happen, using `Future.get()` and handling Exceptions 
sound better plan).
   




----------------------------------------------------------------
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