joshelser commented on a change in pull request #1258: HBASE-23932 Minor
improvements to Region Normalizer
URL: https://github.com/apache/hbase/pull/1258#discussion_r390000985
##########
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:
`toConnection()` returns a cached `Connection` but we make a new `Admin`
object every time (at least via `AsyncClusterConnectionImpl`. Should we just
cache this `Admin` once for all `plans`?
----------------------------------------------------------------
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]
With regards,
Apache Git Services
