From 71af97cd0546e96af1341824c36d36e2eea0dd71 Mon Sep 17 00:00:00 2001 From: Viraj Jasani Date: Fri, 26 Jun 2020 22:08:52 +0530 Subject: [PATCH] HBASE-24588 : Submit task for NormalizationPlan (#1933) Signed-off-by: Nick Dimiduk --- .../org/apache/hadoop/hbase/client/Admin.java | 3 + .../hbase/client/RawAsyncHBaseAdmin.java | 12 +-- .../apache/hadoop/hbase/master/HMaster.java | 73 ++++++++++--------- .../normalizer/EmptyNormalizationPlan.java | 54 -------------- .../normalizer/MergeNormalizationPlan.java | 33 ++++----- .../master/normalizer/NormalizationPlan.java | 12 ++- .../normalizer/SplitNormalizationPlan.java | 24 +++--- 7 files changed, 84 insertions(+), 127 deletions(-) delete mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/EmptyNormalizationPlan.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java index 520a8fc0604..a8d583bec32 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java @@ -833,6 +833,9 @@ public interface Admin extends Abortable, Closeable { /** * Invoke region normalizer. Can NOT run for various reasons. Check logs. + * This is a non-blocking invocation to region normalizer. If return value is true, it means + * the request was submitted successfully. We need to check logs for the details of which regions + * were split/merged. * * @return true if region normalizer ran, false otherwise. * @throws IOException if a remote or network exception occurs diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java index 5ef955ddc43..16483ef98e1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java @@ -1298,7 +1298,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { return; } - MergeTableRegionsRequest request = null; + final MergeTableRegionsRequest request; try { request = RequestConverter.buildMergeTableRegionsRequest(encodedNameOfRegionsToMerge, forcible, ng.getNonceGroup(), ng.newNonce()); @@ -1308,8 +1308,8 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { } addListener( - this. procedureCall(tableName, request, - (s, c, req, done) -> s.mergeTableRegions(c, req, done), (resp) -> resp.getProcId(), + this.procedureCall(tableName, request, + MasterService.Interface::mergeTableRegions, MergeTableRegionsResponse::getProcId, new MergeTableRegionProcedureBiConsumer(tableName)), (ret, err2) -> { if (err2 != null) { @@ -1485,7 +1485,7 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { private CompletableFuture split(final RegionInfo hri, byte[] splitPoint) { CompletableFuture future = new CompletableFuture<>(); TableName tableName = hri.getTable(); - SplitTableRegionRequest request = null; + final SplitTableRegionRequest request; try { request = RequestConverter.buildSplitTableRegionRequest(hri, splitPoint, ng.getNonceGroup(), ng.newNonce()); @@ -1495,8 +1495,8 @@ class RawAsyncHBaseAdmin implements AsyncAdmin { } addListener( - this. procedureCall(tableName, - request, (s, c, req, done) -> s.splitRegion(c, req, done), (resp) -> resp.getProcId(), + this.procedureCall(tableName, + request, MasterService.Interface::splitRegion, SplitTableRegionResponse::getProcId, new SplitTableRegionProcedureBiConsumer(tableName)), (ret, err2) -> { if (err2 != null) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 5a9404a7277..3af98a39672 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -1934,44 +1934,51 @@ public class HMaster extends HRegionServer implements MasterServices { } try { - final List allEnabledTables = new ArrayList<>( - tableStateManager.getTablesInStates(TableState.State.ENABLED)); + final List allEnabledTables = + new ArrayList<>(tableStateManager.getTablesInStates(TableState.State.ENABLED)); Collections.shuffle(allEnabledTables); - try (final Admin admin = asyncClusterConnection.toConnection().getAdmin()) { - for (TableName table : allEnabledTables) { - 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; - } + final List submittedPlanProcIds = new ArrayList<>(); + for (TableName table : allEnabledTables) { + 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; - } + // make one last check that the cluster isn't shutting down before proceeding. + if (skipRegionManagementAction("region normalizer")) { + return false; + } - final List plans = normalizer.computePlansForTable(table); - if (CollectionUtils.isEmpty(plans)) { - LOG.debug("No normalization required for table {}.", table); - continue; - } + final List plans = normalizer.computePlansForTable(table); + if (CollectionUtils.isEmpty(plans)) { + LOG.debug("No normalization required for table {}.", table); + 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. - for (NormalizationPlan plan : plans) { - plan.execute(admin); - if (plan.getType() == PlanType.SPLIT) { - splitPlanCount++; - } else if (plan.getType() == PlanType.MERGE) { - mergePlanCount++; - } + // 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); + submittedPlanProcIds.add(procId); + if (plan.getType() == PlanType.SPLIT) { + splitPlanCount++; + } else if (plan.getType() == PlanType.MERGE) { + mergePlanCount++; } } + int totalPlansSubmitted = submittedPlanProcIds.size(); + if (totalPlansSubmitted > 0 && LOG.isDebugEnabled()) { + LOG.debug("Normalizer plans submitted. Total plans count: {} , procID list: {}", + totalPlansSubmitted, submittedPlanProcIds); + } } } finally { normalizationInProgressLock.unlock(); @@ -2013,8 +2020,8 @@ public class HMaster extends HRegionServer implements MasterServices { " failed because merge switch is off"); } - final String mergeRegionsStr = Arrays.stream(regionsToMerge).map(r -> r.getEncodedName()). - collect(Collectors.joining(", ")); + final String mergeRegionsStr = Arrays.stream(regionsToMerge).map(RegionInfo::getEncodedName) + .collect(Collectors.joining(", ")); return MasterProcedureUtil.submitProcedure(new NonceProcedureRunnable(this, ng, nonce) { @Override protected void run() throws IOException { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/EmptyNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/EmptyNormalizationPlan.java deleted file mode 100644 index a199838a2d6..00000000000 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/EmptyNormalizationPlan.java +++ /dev/null @@ -1,54 +0,0 @@ -/** - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.hadoop.hbase.master.normalizer; - -import org.apache.yetus.audience.InterfaceAudience; -import org.apache.hadoop.hbase.client.Admin; -import org.apache.hadoop.hbase.master.normalizer.NormalizationPlan.PlanType; - -/** - * Plan which signifies that no normalization is required, - * or normalization of this table isn't allowed, this is singleton. - */ -@InterfaceAudience.Private -public final class EmptyNormalizationPlan implements NormalizationPlan { - private static final EmptyNormalizationPlan instance = new EmptyNormalizationPlan(); - - private EmptyNormalizationPlan() { - } - - /** - * @return singleton instance - */ - public static EmptyNormalizationPlan getInstance(){ - return instance; - } - - /** - * No-op for empty plan. - */ - @Override - public void execute(Admin admin) { - } - - @Override - public PlanType getType() { - return PlanType.NONE; - } -} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java index a2938a9f0b1..8f8a43f7e21 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java @@ -20,8 +20,9 @@ package org.apache.hadoop.hbase.master.normalizer; import java.io.IOException; -import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,6 +42,20 @@ public class MergeNormalizationPlan implements NormalizationPlan { this.secondRegion = secondRegion; } + /** + * {@inheritDoc} + */ + @Override + public long submit(MasterServices masterServices) throws IOException { + LOG.info("Executing merging normalization plan: " + this); + // Do not use force=true as corner cases can happen, non adjacent regions, + // merge with a merged child region with no GC done yet, it is going to + // cause all different issues. + return masterServices + .mergeRegions(new RegionInfo[] { firstRegion, secondRegion }, false, HConstants.NO_NONCE, + HConstants.NO_NONCE); + } + @Override public PlanType getType() { return PlanType.MERGE; @@ -62,20 +77,4 @@ public class MergeNormalizationPlan implements NormalizationPlan { '}'; } - /** - * {@inheritDoc} - */ - @Override - public void execute(Admin admin) { - LOG.info("Executing merging normalization plan: " + this); - try { - // Do not use force=true as corner cases can happen, non adjacent regions, - // merge with a merged child region with no GC done yet, it is going to - // cause all different issues. - admin.mergeRegionsAsync(firstRegion.getEncodedNameAsBytes(), - secondRegion.getEncodedNameAsBytes(), false); - } catch (IOException ex) { - LOG.error("Error during region merge: ", ex); - } - } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java index b0541d091e8..cd13f69e764 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/NormalizationPlan.java @@ -18,8 +18,9 @@ */ package org.apache.hadoop.hbase.master.normalizer; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.yetus.audience.InterfaceAudience; -import org.apache.hadoop.hbase.client.Admin; +import java.io.IOException; /** * Interface for normalization plan. @@ -33,10 +34,13 @@ public interface NormalizationPlan { } /** - * Executes normalization plan on cluster (does actual splitting/merging work). - * @param admin instance of Admin + * Submits normalization plan on cluster (does actual splitting/merging work) and + * returns proc Id to caller. + * @param masterServices instance of {@link MasterServices} + * @return Proc Id for the submitted task + * @throws IOException If plan submission to Admin fails */ - void execute(Admin admin); + long submit(MasterServices masterServices) throws IOException; /** * @return the type of this plan diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java index 04e11d15a82..67008b8fed3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SplitNormalizationPlan.java @@ -18,9 +18,11 @@ */ package org.apache.hadoop.hbase.master.normalizer; +import java.io.IOException; import java.util.Arrays; -import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.yetus.audience.InterfaceAudience; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,6 +42,14 @@ public class SplitNormalizationPlan implements NormalizationPlan { this.splitPoint = splitPoint; } + /** + * {@inheritDoc} + */ + @Override + public long submit(MasterServices masterServices) throws IOException { + return masterServices.splitRegion(regionInfo, null, HConstants.NO_NONCE, HConstants.NO_NONCE); + } + @Override public PlanType getType() { return PlanType.SPLIT; @@ -69,16 +79,4 @@ public class SplitNormalizationPlan implements NormalizationPlan { '}'; } - /** - * {@inheritDoc} - */ - @Override - public void execute(Admin admin) { - LOG.info("Executing splitting normalization plan: " + this); - try { - admin.splitRegionAsync(regionInfo.getRegionName()).get(); - } catch (Exception ex) { - LOG.error("Error during region split: ", ex); - } - } }