[GitHub] [hbase] ndimiduk commented on a change in pull request #1258: HBASE-23932 Minor improvements to Region Normalizer

[GitBox <gi...@apache.org>]

ndimiduk commented on a change in pull request #1258: HBASE-23932 Minor improvements to Region Normalizer
URL: https://github.com/apache/hbase/pull/1258#discussion_r390461147
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
 ##########
 @@ -1868,52 +1889,50 @@ public RegionNormalizer getRegionNormalizer() {
    *    is globally disabled)
    */
   public boolean normalizeRegions() throws IOException {
-    if (!isInitialized()) {
-      LOG.debug("Master has not been initialized, don't run region normalizer.");
-      return false;
-    }
-    if (this.getServerManager().isClusterShutdown()) {
-      LOG.info("Cluster is shutting down, don't run region normalizer.");
+    if (regionNormalizerTracker == null || !regionNormalizerTracker.isNormalizerOn()) {
+      LOG.debug("Region normalization is disabled, don't run region normalizer.");
       return false;
     }
-    if (isInMaintenanceMode()) {
-      LOG.info("Master is in maintenance mode, don't run region normalizer.");
+    if (skipRegionManagementAction("region normalizer")) {
       return false;
     }
-    if (!this.regionNormalizerTracker.isNormalizerOn()) {
-      LOG.debug("Region normalization is disabled, don't run region normalizer.");
+    if (assignmentManager.hasRegionsInTransition()) {
       return false;
     }
 
     synchronized (this.normalizer) {
       // Don't run the normalizer concurrently
+
       List<TableName> allEnabledTables = new ArrayList<>(
         this.tableStateManager.getTablesInStates(TableState.State.ENABLED));
 
       Collections.shuffle(allEnabledTables);
 
       for (TableName table : allEnabledTables) {
-        if (isInMaintenanceMode()) {
-          LOG.debug("Master is in maintenance mode, stop running region normalizer.");
-          return false;
-        }
-
         TableDescriptor tblDesc = getTableDescriptors().get(table);
         if (table.isSystemTable() || (tblDesc != null &&
             !tblDesc.isNormalizationEnabled())) {
           LOG.trace("Skipping normalization for {}, as it's either system"
               + " table or doesn't have auto normalization turned on", table);
           continue;
         }
-        List<NormalizationPlan> plans = this.normalizer.computePlanForTable(table);
-        if (plans != null) {
-          for (NormalizationPlan plan : plans) {
-            plan.execute(asyncClusterConnection.toConnection().getAdmin());
-            if (plan.getType() == PlanType.SPLIT) {
-              splitPlanCount++;
-            } else if (plan.getType() == PlanType.MERGE) {
-              mergePlanCount++;
-            }
+
+        // make one last check that the cluster isn't shutting down before proceeding.
+        if (skipRegionManagementAction("region normalizer")) {
+          return false;
+        }
+
+        final List<NormalizationPlan> plans = this.normalizer.computePlanForTable(table);
+        if (CollectionUtils.isEmpty(plans)) {
+          return true;
+        }
+
+        for (NormalizationPlan plan : plans) {
+          plan.execute(asyncClusterConnection.toConnection().getAdmin());
 
 Review comment:
   Oh fun. This code actually leaks admin instances :(

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services