ndimiduk commented on a change in pull request #2454:
URL: https://github.com/apache/hbase/pull/2454#discussion_r498407097
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1924,70 +1907,17 @@ public boolean normalizeRegions(final
NormalizeTableFilterParams ntfp) throws IO
return false;
}
- if (!normalizationInProgressLock.tryLock()) {
- // Don't run the normalizer concurrently
- LOG.info("Normalization already in progress. Skipping request.");
- return true;
- }
-
- int affectedTables = 0;
- try {
- final Set<TableName> matchingTables = getTableDescriptors(new
LinkedList<>(),
- ntfp.getNamespace(), ntfp.getRegex(), ntfp.getTableNames(), false)
- .stream()
- .map(TableDescriptor::getTableName)
- .collect(Collectors.toSet());
- final Set<TableName> allEnabledTables =
- tableStateManager.getTablesInStates(TableState.State.ENABLED);
- final List<TableName> targetTables =
- new ArrayList<>(Sets.intersection(matchingTables, allEnabledTables));
- Collections.shuffle(targetTables);
-
- final List<Long> submittedPlanProcIds = new ArrayList<>();
- for (TableName table : targetTables) {
- if (table.isSystemTable()) {
Review comment:
All this moves to RegionNormalizerWorker.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
##########
@@ -354,6 +353,13 @@ long splitRegion(
*/
boolean isInMaintenanceMode();
+ /**
+ * Checks master state before initiating action over region topology.
+ * @param action the name of the action under consideration, for logging.
+ * @return {@code true} when the caller should exit early, {@code false}
otherwise.
+ */
+ boolean skipRegionManagementAction(final String action);
Review comment:
I don't love exposing this up here on MasterServices, but imo, it's
better to have the interface here than to expose its innards.
Suggestions welcome.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1924,70 +1907,17 @@ public boolean normalizeRegions(final
NormalizeTableFilterParams ntfp) throws IO
return false;
}
- if (!normalizationInProgressLock.tryLock()) {
- // Don't run the normalizer concurrently
- LOG.info("Normalization already in progress. Skipping request.");
- return true;
- }
-
- int affectedTables = 0;
- try {
- final Set<TableName> matchingTables = getTableDescriptors(new
LinkedList<>(),
Review comment:
Table selection code stays here in HMaster.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -464,9 +451,6 @@ public void run() {
// handle table states
private TableStateManager tableStateManager;
- private long splitPlanCount;
Review comment:
Moved to RegionNormalizerWorker.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -203,16 +200,6 @@ public void setMasterServices(final MasterServices
masterServices) {
this.masterServices = masterServices;
}
- @Override
- public void planSkipped(final RegionInfo hri, final PlanType type) {
Review comment:
Moved to RegionNormalizerWorker.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -1924,70 +1907,17 @@ public boolean normalizeRegions(final
NormalizeTableFilterParams ntfp) throws IO
return false;
}
- if (!normalizationInProgressLock.tryLock()) {
- // Don't run the normalizer concurrently
- LOG.info("Normalization already in progress. Skipping request.");
- return true;
- }
-
- int affectedTables = 0;
- try {
- final Set<TableName> matchingTables = getTableDescriptors(new
LinkedList<>(),
- ntfp.getNamespace(), ntfp.getRegex(), ntfp.getTableNames(), false)
- .stream()
- .map(TableDescriptor::getTableName)
- .collect(Collectors.toSet());
- final Set<TableName> allEnabledTables =
- tableStateManager.getTablesInStates(TableState.State.ENABLED);
- final List<TableName> targetTables =
- new ArrayList<>(Sets.intersection(matchingTables, allEnabledTables));
- Collections.shuffle(targetTables);
-
- final List<Long> submittedPlanProcIds = new ArrayList<>();
- for (TableName table : targetTables) {
- if (table.isSystemTable()) {
- continue;
- }
- final TableDescriptor tblDesc = getTableDescriptors().get(table);
- if (tblDesc != null && !tblDesc.isNormalizationEnabled()) {
- LOG.debug(
- "Skipping table {} because normalization is disabled in its table
properties.", table);
- continue;
- }
-
- // make one last check that the cluster isn't shutting down before
proceeding.
- if (skipRegionManagementAction("region normalizer")) {
- return false;
- }
-
- final List<NormalizationPlan> plans =
normalizer.computePlansForTable(table);
- if (CollectionUtils.isEmpty(plans)) {
- LOG.debug("No normalization required for table {}.", table);
- continue;
- }
-
- affectedTables++;
- // 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) {
- long procId = plan.submit(this);
Review comment:
Plan submission is not handled by the worker, the plans are converted
into simple POJOs.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -371,7 +358,11 @@ private boolean skipForMerge(final RegionStates
regionStates, final RegionInfo r
final long nextSizeMb = getRegionSizeMB(next);
// always merge away empty regions when they present themselves.
if (currentSizeMb == 0 || nextSizeMb == 0 || currentSizeMb + nextSizeMb
< avgRegionSizeMb) {
- plans.add(new MergeNormalizationPlan(current, next));
+ final MergeNormalizationPlan plan = new
MergeNormalizationPlan.Builder()
+ .addTarget(current, currentSizeMb)
+ .addTarget(next, nextSizeMb)
Review comment:
This PR is not posted, #2490.
##########
File path:
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -337,9 +330,6 @@ public void run() {
// Tracker for split and merge state
private SplitOrMergeTracker splitOrMergeTracker;
- // Tracker for region normalizer state
- private RegionNormalizerTracker regionNormalizerTracker;
Review comment:
Moved to RegionNormalizerManager.
----------------------------------------------------------------
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]
