From a4d3fc2588b5514459470f13fe949ad10497b682 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Mon, 12 Mar 2018 12:15:24 -0700 Subject: [PATCH] HBASE-20178 [AMv2] Throw exception if hostile environment Fail-fast by throwing exception out of Procedure constructor if move but table is disabled, etc. M hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java Test stolen from HBASE-20131 M hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java Add convenience isEnabled/isDisabled M hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java Promote assert state to throw exception. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java Add isClusterUp M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java Move constructor now throws exception M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/SplitTableRegionProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java Do environment check at construction and fail-fast if hostile. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java Ad checkTableOnline utility method. M hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/GCMergedRegionsProcedure.java Moved to procedure package. --- .../org/apache/hadoop/hbase/client/TableState.java | 14 +++ .../apache/hadoop/hbase/master/CatalogJanitor.java | 2 +- .../org/apache/hadoop/hbase/master/HMaster.java | 1 - .../apache/hadoop/hbase/master/MasterServices.java | 5 + .../hbase/master/assignment/AssignProcedure.java | 2 +- .../hbase/master/assignment/AssignmentManager.java | 10 +- .../assignment/MergeTableRegionsProcedure.java | 7 +- .../master/assignment/MoveRegionProcedure.java | 12 +- .../assignment/SplitTableRegionProcedure.java | 3 + .../AbstractStateMachineTableProcedure.java | 30 +++++ .../master/procedure/DisableTableProcedure.java | 8 +- .../GCMergedRegionsProcedure.java | 5 +- .../master/procedure/ModifyTableProcedure.java | 8 +- .../master/procedure/RestoreSnapshotProcedure.java | 8 +- .../master/procedure/TruncateTableProcedure.java | 8 +- .../hadoop/hbase/regionserver/HRegionServer.java | 3 +- .../hbase/regionserver/RegionServerServices.java | 5 + .../hadoop/hbase/MockRegionServerServices.java | 5 + .../hbase/master/MockNoopMasterServices.java | 5 + .../hadoop/hbase/master/MockRegionServer.java | 5 + .../hadoop/hbase/regionserver/TestRegionMove.java | 130 +++++++++++++++++++++ 21 files changed, 254 insertions(+), 22 deletions(-) rename hbase-server/src/main/java/org/apache/hadoop/hbase/master/{assignment => procedure}/GCMergedRegionsProcedure.java (96%) create mode 100644 hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java index d2334a4266..1df07cc1a5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/TableState.java @@ -39,6 +39,20 @@ public class TableState { ENABLING; /** + * @return True if table is enabled: i.e. {@link #ENABLED} or {@link #ENABLING}. + */ + public boolean isEnabled() { + return isInStates(ENABLED, ENABLING); + } + + /** + * @return True if table is disabled: i.e. NOT {@link #ENABLED} and NOT {@link #ENABLING}. + */ + public boolean isDisabled() { + return !isEnabled(); + } + + /** * Covert from PB version of State * * @param state convert from diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java index 23912d67c8..2a20ef8587 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java @@ -39,7 +39,7 @@ import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.Result; import org.apache.hadoop.hbase.client.TableDescriptor; import org.apache.hadoop.hbase.master.assignment.AssignmentManager; -import org.apache.hadoop.hbase.master.assignment.GCMergedRegionsProcedure; +import org.apache.hadoop.hbase.master.procedure.GCMergedRegionsProcedure; import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.procedure2.ProcedureExecutor; 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 cd967ed4d8..bb11b79781 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 @@ -1681,7 +1681,6 @@ public class HMaster extends HRegionServer implements MasterServices { // Now we can do the move RegionPlan rp = new RegionPlan(hri, regionState.getServerName(), dest); assert rp.getDestination() != null: rp.toString() + " " + dest; - assert rp.getSource() != null: rp.toString(); try { checkInitialized(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java index 0e552d6930..52046c5149 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java @@ -501,4 +501,9 @@ public interface MasterServices extends Server { boolean recoverMeta() throws IOException; String getClientIdAuditPrefix(); + + /** + * @return True if cluster is up; false if cluster is not up (we are shutting down). + */ + boolean isClusterUp(); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java index fd3155398f..2de412f378 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java @@ -161,7 +161,7 @@ public class AssignProcedure extends RegionTransitionProcedure { // Don't assign if table is in disabling or disabled state. TableStateManager tsm = env.getMasterServices().getTableStateManager(); TableName tn = regionNode.getRegionInfo().getTable(); - if (tsm.isTableState(tn, TableState.State.DISABLING, TableState.State.DISABLED)) { + if (!tsm.getTableState(tn).isDisabled()) { LOG.info("Table " + tn + " state=" + tsm.getTableState(tn) + ", skipping " + this); return false; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java index 33a8d2174b..28896f11d3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java @@ -556,7 +556,7 @@ public class AssignmentManager implements ServerListener { ProcedureSyncWait.submitAndWaitProcedure(master.getMasterProcedureExecutor(), proc); } - public Future moveAsync(final RegionPlan regionPlan) { + public Future moveAsync(final RegionPlan regionPlan) throws HBaseIOException { MoveRegionProcedure proc = createMoveRegionProcedure(regionPlan); return ProcedureSyncWait.submitProcedure(master.getMasterProcedureExecutor(), proc); } @@ -678,7 +678,7 @@ public class AssignmentManager implements ServerListener { return procedures.toArray(ASSIGN_PROCEDURE_ARRAY_TYPE); } - // Needed for the following method so it can type the created Array we return + // Needed for the following method so it can type the created Array we retur n private static final UnassignProcedure [] UNASSIGN_PROCEDURE_ARRAY_TYPE = new UnassignProcedure[0]; @@ -695,7 +695,8 @@ public class AssignmentManager implements ServerListener { return procs.toArray(UNASSIGN_PROCEDURE_ARRAY_TYPE); } - public MoveRegionProcedure[] createReopenProcedures(final Collection regionInfo) { + public MoveRegionProcedure[] createReopenProcedures(final Collection regionInfo) + throws IOException { final MoveRegionProcedure[] procs = new MoveRegionProcedure[regionInfo.size()]; int index = 0; for (RegionInfo hri: regionInfo) { @@ -744,7 +745,8 @@ public class AssignmentManager implements ServerListener { return proc; } - public MoveRegionProcedure createMoveRegionProcedure(final RegionPlan plan) { + public MoveRegionProcedure createMoveRegionProcedure(final RegionPlan plan) + throws HBaseIOException { if (plan.getRegionInfo().getTable().isSystemTable()) { List exclude = getExcludedServersForSystemTable(); if (plan.getDestination() != null && exclude.contains(plan.getDestination())) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java index 052ba7fbd3..ec59d15bfd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java @@ -73,6 +73,8 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.M * The procedure to Merge a region in a table. * This procedure takes an exclusive table lock since it is working over multiple regions. * It holds the lock for the life of the procedure. + *

Throws exception on construction if determines context hostile to merge (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class MergeTableRegionsProcedure @@ -96,14 +98,15 @@ public class MergeTableRegionsProcedure public MergeTableRegionsProcedure(final MasterProcedureEnv env, final RegionInfo regionToMergeA, final RegionInfo regionToMergeB, - final boolean forcible) throws MergeRegionException { + final boolean forcible) throws IOException { this(env, new RegionInfo[] {regionToMergeA, regionToMergeB}, forcible); } public MergeTableRegionsProcedure(final MasterProcedureEnv env, final RegionInfo[] regionsToMerge, final boolean forcible) - throws MergeRegionException { + throws IOException { super(env); + checkTableOnline(env); // Check daughter regions and make sure that we have valid daughter regions // before doing the real work. diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java index a29bfee2cb..ef481082ac 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MoveRegionProcedure.java @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master.assignment; import java.io.IOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; @@ -40,6 +41,9 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.MasterProcedureProtos.M * It first runs an unassign subprocedure followed * by an assign subprocedure. It takes a lock on the region being moved. * It holds the lock for the life of the procedure. + * + *

Throws exception on construction if determines context hostile to move (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class MoveRegionProcedure extends AbstractStateMachineRegionProcedure { @@ -51,9 +55,15 @@ public class MoveRegionProcedure extends AbstractStateMachineRegionProcedureThrows exception on construction if determines context hostile to spllt (cluster going + * down or master is shutting down or table is disabled).

*/ @InterfaceAudience.Private public class SplitTableRegionProcedure @@ -104,6 +106,7 @@ public class SplitTableRegionProcedure public SplitTableRegionProcedure(final MasterProcedureEnv env, final RegionInfo regionToSplit, final byte[] splitRow) throws IOException { super(env, regionToSplit); + checkTableOnline(env); this.bestSplitRow = splitRow; checkSplittable(env, regionToSplit, bestSplitRow); final TableName table = regionToSplit.getTable(); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java index 833b659a5b..607d2127ff 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/AbstractStateMachineTableProcedure.java @@ -20,11 +20,14 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotEnabledException; import org.apache.hadoop.hbase.TableNotFoundException; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.master.MasterFileSystem; +import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.procedure2.StateMachineProcedure; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.FSUtils; @@ -124,4 +127,31 @@ public abstract class AbstractStateMachineTableProcedure Path tableDir = FSUtils.getTableDir(mfs.getRootDir(), getTableName()); return new Path(tableDir, ServerRegionReplicaUtil.getRegionInfoForFs(region).getEncodedName()); } + + /** + * Check table is online and modifiable and that cluster and master are up. + * Run in constructor so can pass exception to caller. + */ + protected void checkTableOnline(MasterProcedureEnv env) throws HBaseIOException { + MasterServices master = env.getMasterServices(); + if (!master.isClusterUp()) { + throw new HBaseIOException("Cluster not up!"); + } + if (master.isStopping() || master.isStopped()) { + throw new HBaseIOException("Master stopping=" + master.isStopping() + + ", stopped=" + master.isStopped()); + } + try { + checkTableModifiable(env); + if (env.getMasterServices().getTableStateManager().getTableState(getTableName()). + isDisabled()) { + throw new TableNotEnabledException(getTableName()); + } + } catch (IOException ioe) { + if (ioe instanceof HBaseIOException) { + throw (HBaseIOException)ioe; + } + throw new HBaseIOException(ioe); + } + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java index e748c6ce7f..7e657e1322 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/DisableTableProcedure.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotEnabledException; @@ -57,7 +58,8 @@ public class DisableTableProcedure * @param skipTableStateCheck whether to check table state */ public DisableTableProcedure(final MasterProcedureEnv env, final TableName tableName, - final boolean skipTableStateCheck) { + final boolean skipTableStateCheck) + throws HBaseIOException { this(env, tableName, skipTableStateCheck, null); } @@ -68,9 +70,11 @@ public class DisableTableProcedure * @param skipTableStateCheck whether to check table state */ public DisableTableProcedure(final MasterProcedureEnv env, final TableName tableName, - final boolean skipTableStateCheck, final ProcedurePrepareLatch syncLatch) { + final boolean skipTableStateCheck, final ProcedurePrepareLatch syncLatch) + throws HBaseIOException { super(env, syncLatch); this.tableName = tableName; + checkTableOnline(env); this.skipTableStateCheck = skipTableStateCheck; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/GCMergedRegionsProcedure.java similarity index 96% rename from hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java rename to hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/GCMergedRegionsProcedure.java index 610003df91..97bcb21ebf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/GCMergedRegionsProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/GCMergedRegionsProcedure.java @@ -15,15 +15,14 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package org.apache.hadoop.hbase.master.assignment; +package org.apache.hadoop.hbase.master.procedure; import java.io.IOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.master.procedure.AbstractStateMachineTableProcedure; -import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; +import org.apache.hadoop.hbase.master.assignment.GCRegionProcedure; import org.apache.hadoop.hbase.procedure2.ProcedureStateSerializer; import org.apache.hadoop.hbase.procedure2.ProcedureSuspendedException; import org.apache.hadoop.hbase.procedure2.ProcedureYieldException; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java index f0be1e0819..bf6233c602 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ModifyTableProcedure.java @@ -24,6 +24,7 @@ import java.util.List; import java.util.Set; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; @@ -63,15 +64,18 @@ public class ModifyTableProcedure initilize(); } - public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd) { + public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd) + throws HBaseIOException { this(env, htd, null); } public ModifyTableProcedure(final MasterProcedureEnv env, final TableDescriptor htd, - final ProcedurePrepareLatch latch) { + final ProcedurePrepareLatch latch) + throws HBaseIOException { super(env, latch); initilize(); this.modifiedTableDescriptor = htd; + checkTableOnline(env); } private void initilize() { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java index 9aa5171786..b1216af9e3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java @@ -28,6 +28,7 @@ import java.util.Map; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.MetaTableAccessor; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotFoundException; @@ -81,7 +82,8 @@ public class RestoreSnapshotProcedure } public RestoreSnapshotProcedure(final MasterProcedureEnv env, - final TableDescriptor tableDescriptor, final SnapshotDescription snapshot) { + final TableDescriptor tableDescriptor, final SnapshotDescription snapshot) + throws HBaseIOException { this(env, tableDescriptor, snapshot, false); } /** @@ -95,10 +97,12 @@ public class RestoreSnapshotProcedure final MasterProcedureEnv env, final TableDescriptor tableDescriptor, final SnapshotDescription snapshot, - final boolean restoreAcl) { + final boolean restoreAcl) + throws HBaseIOException { super(env); // This is the new schema we are going to write out as this modification. this.modifiedTableDescriptor = tableDescriptor; + checkTableOnline(env); // Snapshot information this.snapshot = snapshot; this.restoreAcl = restoreAcl; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java index 80cc5a8047..bae93a21bf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/TruncateTableProcedure.java @@ -23,6 +23,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.TableNotDisabledException; import org.apache.hadoop.hbase.TableNotFoundException; @@ -56,14 +57,17 @@ public class TruncateTableProcedure } public TruncateTableProcedure(final MasterProcedureEnv env, final TableName tableName, - boolean preserveSplits) { + boolean preserveSplits) + throws HBaseIOException { this(env, tableName, preserveSplits, null); } public TruncateTableProcedure(final MasterProcedureEnv env, final TableName tableName, - boolean preserveSplits, ProcedurePrepareLatch latch) { + boolean preserveSplits, ProcedurePrepareLatch latch) + throws HBaseIOException { super(env, latch); this.tableName = tableName; + checkTableOnline(env); this.preserveSplits = preserveSplits; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 81febc0ea9..e03757ffbd 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -910,7 +910,8 @@ public class HRegionServer extends HasThread implements /** * @return True if the cluster is up. */ - private boolean isClusterUp() { + @Override + public boolean isClusterUp() { return this.masterless || (this.clusterStatusTracker != null && this.clusterStatusTracker.isClusterUp()); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java index 60bd215933..023efd94b9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java @@ -234,4 +234,9 @@ public interface RegionServerServices extends Server, MutableOnlineRegions, Favo * See HBASE-17712 for more details. */ void unassign(byte[] regionName) throws IOException; + + /** + * @return True if cluster is up; false if cluster is not up (we are shutting down). + */ + boolean isClusterUp(); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java index f1e020f581..879b592f17 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java @@ -334,4 +334,9 @@ public class MockRegionServerServices implements RegionServerServices { public Connection createConnection(Configuration conf) throws IOException { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java index 4e66676303..cc2f43dd44 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockNoopMasterServices.java @@ -478,4 +478,9 @@ public class MockNoopMasterServices implements MasterServices { public ReplicationPeerManager getReplicationPeerManager() { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } \ No newline at end of file diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java index cabd769698..d366b67fe5 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java @@ -678,4 +678,9 @@ ClientProtos.ClientService.BlockingInterface, RegionServerServices { public Connection createConnection(Configuration conf) throws IOException { return null; } + + @Override + public boolean isClusterUp() { + return true; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java new file mode 100644 index 0000000000..eb7858e34b --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMove.java @@ -0,0 +1,130 @@ +/** + * 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.regionserver; + +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.util.List; +import java.util.stream.Collectors; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.DoNotRetryIOException; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.TableNotEnabledException; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.testclassification.LargeTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.AfterClass; +import org.junit.Before; +import org.junit.BeforeClass; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; +import org.junit.rules.TestName; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * Test move fails when table disabled + */ +@Category({LargeTests.class}) +public class TestRegionMove { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestRegionMove.class); + + @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Rule + public TestName name = new TestName(); + private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility(); + public static Configuration CONF ; + protected static final String F1 = "f1"; + + // Test names + protected TableName tableName; + protected String method; + + @BeforeClass + public static void startCluster() throws Exception { + TEST_UTIL.startMiniCluster(2); + } + + @AfterClass + public static void stopCluster() throws Exception { + TEST_UTIL.shutdownMiniCluster(); + } + + @Before + public void setup() throws IOException { + CONF = TEST_UTIL.getConfiguration(); + method = name.getMethodName(); + tableName = TableName.valueOf(method); + } + + @Test + public void testDisableAndMove() throws Exception { + Admin admin = TEST_UTIL.getAdmin(); + + // Create a table with more than one region + Table t = TEST_UTIL.createMultiRegionTable(tableName, Bytes.toBytes(F1), 10); + TEST_UTIL.waitUntilAllRegionsAssigned(tableName); + + // Write an update to each region + for (RegionInfo regionInfo : admin.getRegions(tableName)) { + byte[] startKey = regionInfo.getStartKey(); + // The startKey of the first region is "empty", which would throw an error if we try to Put that. + byte[] rowKey = org.apache.hbase.thirdparty.com.google.common.primitives.Bytes.concat( + startKey, Bytes.toBytes("1")); + Put p = new Put(rowKey); + p.addColumn(Bytes.toBytes(F1), Bytes.toBytes("q1"), Bytes.toBytes("value")); + t.put(p); + } + + // Get a Region which is on the first RS + HRegionServer rs1 = TEST_UTIL.getRSForFirstRegionInTable(tableName); + HRegionServer rs2 = TEST_UTIL.getOtherRegionServer(rs1); + List regionsOnRS1ForTable = admin.getRegions(rs1.getServerName()).stream() + .filter((regionInfo) -> regionInfo.getTable().equals(tableName)) + .collect(Collectors.toList()); + assertTrue( + "Expected to find at least one region for " + tableName + " on " + rs1.getServerName() + + ", but found none", !regionsOnRS1ForTable.isEmpty()); + final RegionInfo regionToMove = regionsOnRS1ForTable.get(0); + + // Disable the table + admin.disableTable(tableName); + + // We except a DNRIOE when we try to move a region which isn't open. + thrown.expect(TableNotEnabledException.class); + thrown.expectMessage(t.getName().toString()); + + // Move the region to the other RS -- should fail + admin.move(regionToMove.getEncodedNameAsBytes(), Bytes.toBytes(rs2.getServerName().toString())); + } +} -- 2.11.0 (Apple Git-81)