huaxiangsun commented on a change in pull request #1933:
URL: https://github.com/apache/hbase/pull/1933#discussion_r443875648
##########
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();
Review comment:
As Nick commented, need to think about the purpose of this change. I
think the purpose is to know the result of plans, which comes with cost.
1). Timeout with Future.get().
When it times out, we do not know if plan succeeds or not. The only info
it gives us is that the plan does not finish within a certain amount of time.
2). Perform all.get() asynchronously.
If get() blocks for whatever reason, there will be huge number of
threads blocking for get(), system resource leak.
Maybe the best approach is per Nick's comments, log how many plans
submitted. If some plans fail, we can go to procedure system for root cause.
----------------------------------------------------------------
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]
