From 87fc1435c75fb9fb0e2db15922beb7a7b6c5bc64 Mon Sep 17 00:00:00 2001 From: Wellington Chevreuil Date: Thu, 5 Jan 2023 09:06:45 +0000 Subject: [PATCH 1/5] HBASE-27551 Add config options to delay assignment to retain last region location --- .../hadoop/hbase/master/ServerManager.java | 2 +- .../TransitRegionStateProcedure.java | 61 ++++++++++++++- .../master/TestRetainAssignmentOnRestart.java | 75 +++++++++++++++++++ 3 files changed, 136 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java index ed28db78de71..6a169beb53bf 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java @@ -406,7 +406,7 @@ private void checkIsDead(final ServerName serverName, final String what) * Assumes onlineServers is locked. * @return ServerName with matching hostname and port. */ - private ServerName findServerWithSameHostnamePortWithLock(final ServerName serverName) { + public ServerName findServerWithSameHostnamePortWithLock(final ServerName serverName) { ServerName end = ServerName.valueOf(serverName.getHostname(), serverName.getPort(), Long.MAX_VALUE); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 72ac2d3827fd..65431db3561a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -31,6 +31,7 @@ import org.apache.hadoop.hbase.client.RetriesExhaustedException; import org.apache.hadoop.hbase.master.MetricsAssignmentManager; import org.apache.hadoop.hbase.master.RegionState.State; +import org.apache.hadoop.hbase.master.ServerManager; import org.apache.hadoop.hbase.master.procedure.AbstractStateMachineRegionProcedure; import org.apache.hadoop.hbase.master.procedure.MasterProcedureEnv; import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure; @@ -107,6 +108,20 @@ public class TransitRegionStateProcedure private static final Logger LOG = LoggerFactory.getLogger(TransitRegionStateProcedure.class); + public static final String FORCE_REGION_RETAINMENT = "hbase.master.scp.retain.assignment.force"; + + public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; + + public static final String FORCE_REGION_RETAINMENT_WAIT = + "hbase.master.scp.retain.assignment.force.wait"; + + public static final long DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 500; + + public static final String FORCE_REGION_RETAINMENT_RETRIES = + "hbase.master.scp.retain.assignment.force.retries"; + + public static final long DEFAULT_FORCE_REGION_RETAINMENT_RETRIES = 600; + private TransitionType type; private RegionStateTransitionState initialState; @@ -126,6 +141,14 @@ public class TransitRegionStateProcedure private boolean isSplit; + private boolean forceRegionRetainment; + + private ServerManager serverManager; + + private long forceRegionRetainmentWait; + + private long forceRegionRetainmentRetries; + public TransitRegionStateProcedure() { } @@ -163,6 +186,17 @@ protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, } evictCache = env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE); + + forceRegionRetainment = env.getMasterConfiguration().getBoolean(FORCE_REGION_RETAINMENT, + DEFAULT_FORCE_REGION_RETAINMENT); + + forceRegionRetainmentWait = env.getMasterConfiguration().getLong(FORCE_REGION_RETAINMENT_WAIT, + DEFAULT_FORCE_REGION_RETAINMENT_WAIT); + + forceRegionRetainmentRetries = env.getMasterConfiguration() + .getLong(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); + + serverManager = env.getMasterServices().getServerManager(); } protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, @@ -188,6 +222,25 @@ protected boolean waitInitialized(MasterProcedureEnv env) { return am.waitMetaLoaded(this) || am.waitMetaAssigned(this, getRegion()); } + private void checkAndWaitForOriginalServer(ServerName lastHost) + throws ProcedureSuspendedException { + boolean isOnline = serverManager.findServerWithSameHostnamePortWithLock(lastHost) != null; + long retries = 0; + while (!isOnline && retries < forceRegionRetainmentRetries) { + try { + Thread.sleep(forceRegionRetainmentWait); + } catch (InterruptedException e) { + throw new ProcedureSuspendedException(); + } + retries++; + isOnline = serverManager.findServerWithSameHostnamePortWithLock(lastHost) != null; + } + LOG.info( + "{} is true. We waited {} ms for host {} to come back online. " + + "Did host come back online? {}", + FORCE_REGION_RETAINMENT, (retries * forceRegionRetainmentRetries), lastHost, isOnline); + } + private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) throws ProcedureSuspendedException { boolean retain = false; @@ -200,9 +253,15 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) regionNode.setRegionLocation(assignCandidate); } else if (regionNode.getLastHost() != null) { retain = true; - LOG.info("Setting lastHost as the region location {}", regionNode.getLastHost()); + LOG.info("Setting lastHost {} as the location for region {}", regionNode.getLastHost(), + regionNode.getRegionInfo().getEncodedName()); regionNode.setRegionLocation(regionNode.getLastHost()); } + if (regionNode.getRegionLocation() != null && forceRegionRetainment) { + LOG.warn("{} is set to true. This may delay regions re-assignment " + + "upon RegionServers crashes or restarts.", FORCE_REGION_RETAINMENT); + checkAndWaitForOriginalServer(regionNode.getRegionLocation()); + } } LOG.info("Starting {}; {}; forceNewPlan={}, retain={}", this, regionNode.toShortString(), forceNewPlan, retain); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java index 641a07315ff2..c846be916971 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java @@ -17,6 +17,8 @@ */ package org.apache.hadoop.hbase.master; +import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT; +import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT_WAIT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; @@ -228,6 +230,79 @@ public void testRetainAssignmentOnSingleRSRestart() throws Exception { } } + /** + * This tests the force retaining assignments upon an RS restart, even when master triggers an SCP + */ + @Test + public void testForceRetainAssignment() throws Exception { + UTIL.getConfiguration().setBoolean(FORCE_REGION_RETAINMENT, true); + UTIL.getConfiguration().setLong(FORCE_REGION_RETAINMENT_WAIT, 2000); + setupCluster(); + HMaster master = UTIL.getMiniHBaseCluster().getMaster(); + SingleProcessHBaseCluster cluster = UTIL.getHBaseCluster(); + List threads = cluster.getLiveRegionServerThreads(); + assertEquals(NUM_OF_RS, threads.size()); + int[] rsPorts = new int[NUM_OF_RS]; + for (int i = 0; i < NUM_OF_RS; i++) { + rsPorts[i] = threads.get(i).getRegionServer().getServerName().getPort(); + } + + // We don't have to use SnapshotOfRegionAssignmentFromMeta. We use it here because AM used to + // use it to load all user region placements + SnapshotOfRegionAssignmentFromMeta snapshot = + new SnapshotOfRegionAssignmentFromMeta(master.getConnection()); + snapshot.initialize(); + Map regionToRegionServerMap = snapshot.getRegionToRegionServerMap(); + for (ServerName serverName : regionToRegionServerMap.values()) { + boolean found = false; // Test only, no need to optimize + for (int k = 0; k < NUM_OF_RS && !found; k++) { + found = serverName.getPort() == rsPorts[k]; + } + assertTrue(found); + } + + // Server to be restarted + ServerName deadRS = threads.get(0).getRegionServer().getServerName(); + List deadRSRegions = snapshot.getRegionServerToRegionMap().get(deadRS); + LOG.info("\n\nStopping {} server", deadRS); + cluster.stopRegionServer(deadRS); + + LOG.info("\n\nSleeping a bit"); + Thread.sleep(1000); + + LOG.info("\n\nStarting region server {} second time with the same port", deadRS); + cluster.getConf().setInt(ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, 3); + cluster.getConf().setInt(HConstants.REGIONSERVER_PORT, deadRS.getPort()); + cluster.startRegionServer(); + + ensureServersWithSamePort(master, rsPorts); + + // Wait till master is initialized and all regions are assigned + for (TableName TABLE : TABLES) { + UTIL.waitTableAvailable(TABLE); + } + UTIL.waitUntilNoRegionsInTransition(60000); + + snapshot = new SnapshotOfRegionAssignmentFromMeta(master.getConnection()); + snapshot.initialize(); + Map newRegionToRegionServerMap = snapshot.getRegionToRegionServerMap(); + assertEquals(regionToRegionServerMap.size(), newRegionToRegionServerMap.size()); + for (Map.Entry entry : newRegionToRegionServerMap.entrySet()) { + ServerName oldServer = regionToRegionServerMap.get(entry.getKey()); + ServerName currentServer = entry.getValue(); + LOG.info( + "Key=" + entry.getKey() + " oldServer=" + oldServer + ", currentServer=" + currentServer); + assertEquals(entry.getKey().toString(), oldServer.getAddress(), currentServer.getAddress()); + + if (deadRS.getPort() == oldServer.getPort()) { + // Restarted RS start code wont be same + assertNotEquals(oldServer.getStartcode(), currentServer.getStartcode()); + } else { + assertEquals(oldServer.getStartcode(), currentServer.getStartcode()); + } + } + } + private void setupCluster() throws Exception, IOException, InterruptedException { // Set Zookeeper based connection registry since we will stop master and start a new master // without populating the underlying config for the connection. From 69db293328b740399c0d3eafba620e76da066a8c Mon Sep 17 00:00:00 2001 From: Wellington Chevreuil Date: Mon, 9 Jan 2023 15:21:45 +0000 Subject: [PATCH 2/5] Added comments to the two new config properties Change-Id: I58eba4f382974f73167fc8cd2bf1bb4f3d5be0a9 --- .../hbase/master/assignment/TransitRegionStateProcedure.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 65431db3561a..51f4787a8ed9 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -112,9 +112,12 @@ public class TransitRegionStateProcedure public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; + /** The wait time in millis before checking again if the region's previous RS is back online*/ public static final String FORCE_REGION_RETAINMENT_WAIT = "hbase.master.scp.retain.assignment.force.wait"; + /** The number of times to check if the region's previous RS is back online, before giving up + * and proceeding with assignment on a new RS*/ public static final long DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 500; public static final String FORCE_REGION_RETAINMENT_RETRIES = From d1c9ecbdd845125e593303540f0ef032fa83cf3c Mon Sep 17 00:00:00 2001 From: Wellington Chevreuil Date: Thu, 12 Jan 2023 12:08:07 +0000 Subject: [PATCH 3/5] Addressing Duo's suggestions Change-Id: Ie13da5dfad3969c1477eac0e37f808051c884bff --- .../TransitRegionStateProcedure.java | 54 ++++++++++++------- .../master/TestRetainAssignmentOnRestart.java | 11 ++-- 2 files changed, 39 insertions(+), 26 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 51f4787a8ed9..7d196bc775c1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -96,6 +96,10 @@ * Notice that, although we allow specify a target server, it just acts as a candidate, we do not * guarantee that the region will finally be on the target server. If this is important for you, you * should check whether the region is on the target server after the procedure is finished. + *

+ * Altenatively, for trying retaining assignments, the + * hbase.master.scp.retain.assignment.force option can be used together with + * hbase.master.scp.retain.assignment. *

* When you want to schedule a TRSP, please check whether there is still one for this region, and * the check should be under the RegionStateNode lock. We will remove the TRSP from a @@ -112,14 +116,16 @@ public class TransitRegionStateProcedure public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; - /** The wait time in millis before checking again if the region's previous RS is back online*/ + /** The wait time in millis before checking again if the region's previous RS is back online */ public static final String FORCE_REGION_RETAINMENT_WAIT = "hbase.master.scp.retain.assignment.force.wait"; - /** The number of times to check if the region's previous RS is back online, before giving up - * and proceeding with assignment on a new RS*/ - public static final long DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 500; + public static final int DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 100; + /** + * The number of times to check if the region's previous RS is back online, before giving up and + * proceeding with assignment on a new RS + */ public static final String FORCE_REGION_RETAINMENT_RETRIES = "hbase.master.scp.retain.assignment.force.retries"; @@ -148,10 +154,12 @@ public class TransitRegionStateProcedure private ServerManager serverManager; - private long forceRegionRetainmentWait; + private int forceRegionRetainmentWait; private long forceRegionRetainmentRetries; + private long retries; + public TransitRegionStateProcedure() { } @@ -190,18 +198,24 @@ protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, evictCache = env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE); + readConfigs(env); + } + + private void readConfigs(MasterProcedureEnv env) { forceRegionRetainment = env.getMasterConfiguration().getBoolean(FORCE_REGION_RETAINMENT, DEFAULT_FORCE_REGION_RETAINMENT); - - forceRegionRetainmentWait = env.getMasterConfiguration().getLong(FORCE_REGION_RETAINMENT_WAIT, + forceRegionRetainmentWait = env.getMasterConfiguration().getInt(FORCE_REGION_RETAINMENT_WAIT, DEFAULT_FORCE_REGION_RETAINMENT_WAIT); - forceRegionRetainmentRetries = env.getMasterConfiguration() .getLong(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); - serverManager = env.getMasterServices().getServerManager(); } + @Override + protected void afterReplay(MasterProcedureEnv env) { + readConfigs(env); + } + protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, ServerName assignCandidate, boolean forceNewPlan, TransitionType type, boolean isSplit) { this(env, hri, assignCandidate, forceNewPlan, type); @@ -227,21 +241,21 @@ protected boolean waitInitialized(MasterProcedureEnv env) { private void checkAndWaitForOriginalServer(ServerName lastHost) throws ProcedureSuspendedException { - boolean isOnline = serverManager.findServerWithSameHostnamePortWithLock(lastHost) != null; - long retries = 0; - while (!isOnline && retries < forceRegionRetainmentRetries) { - try { - Thread.sleep(forceRegionRetainmentWait); - } catch (InterruptedException e) { - throw new ProcedureSuspendedException(); - } + ServerName newNameForServer = serverManager.findServerWithSameHostnamePortWithLock(lastHost); + boolean isOnline = serverManager.createDestinationServersList().contains(newNameForServer); + if (!isOnline && retries < forceRegionRetainmentRetries) { retries++; - isOnline = serverManager.findServerWithSameHostnamePortWithLock(lastHost) != null; + LOG.info("Suspending the TRSP PID={} because {} is true and previous host {} " + + "for region is not yet online.", this.getProcId(), FORCE_REGION_RETAINMENT, lastHost); + setTimeout(forceRegionRetainmentWait); + setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT); + throw new ProcedureSuspendedException(); } LOG.info( - "{} is true. We waited {} ms for host {} to come back online. " + "{} is true. TRSP PID={} waited {}ms for host {} to come back online. " + "Did host come back online? {}", - FORCE_REGION_RETAINMENT, (retries * forceRegionRetainmentRetries), lastHost, isOnline); + FORCE_REGION_RETAINMENT, this.getProcId(), (retries * forceRegionRetainmentWait), lastHost, + isOnline); } private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java index c846be916971..52c2aea884ec 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java @@ -19,6 +19,7 @@ import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT; import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT_WAIT; +import static org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure.MASTER_SCP_RETAIN_ASSIGNMENT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; @@ -35,7 +36,6 @@ import org.apache.hadoop.hbase.StartTestingClusterOption; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure; import org.apache.hadoop.hbase.testclassification.MasterTests; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.util.JVMClusterUtil; @@ -236,7 +236,7 @@ public void testRetainAssignmentOnSingleRSRestart() throws Exception { @Test public void testForceRetainAssignment() throws Exception { UTIL.getConfiguration().setBoolean(FORCE_REGION_RETAINMENT, true); - UTIL.getConfiguration().setLong(FORCE_REGION_RETAINMENT_WAIT, 2000); + UTIL.getConfiguration().setLong(FORCE_REGION_RETAINMENT_WAIT, 100); setupCluster(); HMaster master = UTIL.getMiniHBaseCluster().getMaster(); SingleProcessHBaseCluster cluster = UTIL.getHBaseCluster(); @@ -258,17 +258,17 @@ public void testForceRetainAssignment() throws Exception { for (int k = 0; k < NUM_OF_RS && !found; k++) { found = serverName.getPort() == rsPorts[k]; } + LOG.info("Server {} has regions? {}", serverName, found); assertTrue(found); } // Server to be restarted ServerName deadRS = threads.get(0).getRegionServer().getServerName(); - List deadRSRegions = snapshot.getRegionServerToRegionMap().get(deadRS); LOG.info("\n\nStopping {} server", deadRS); cluster.stopRegionServer(deadRS); LOG.info("\n\nSleeping a bit"); - Thread.sleep(1000); + Thread.sleep(2000); LOG.info("\n\nStarting region server {} second time with the same port", deadRS); cluster.getConf().setInt(ServerManager.WAIT_ON_REGIONSERVERS_MINTOSTART, 3); @@ -282,7 +282,6 @@ public void testForceRetainAssignment() throws Exception { UTIL.waitTableAvailable(TABLE); } UTIL.waitUntilNoRegionsInTransition(60000); - snapshot = new SnapshotOfRegionAssignmentFromMeta(master.getConnection()); snapshot.initialize(); Map newRegionToRegionServerMap = snapshot.getRegionToRegionServerMap(); @@ -309,7 +308,7 @@ private void setupCluster() throws Exception, IOException, InterruptedException UTIL.getConfiguration().set(HConstants.CLIENT_CONNECTION_REGISTRY_IMPL_CONF_KEY, HConstants.ZK_CONNECTION_REGISTRY_CLASS); // Enable retain assignment during ServerCrashProcedure - UTIL.getConfiguration().setBoolean(ServerCrashProcedure.MASTER_SCP_RETAIN_ASSIGNMENT, true); + UTIL.getConfiguration().setBoolean(MASTER_SCP_RETAIN_ASSIGNMENT, true); UTIL.startMiniCluster(StartTestingClusterOption.builder().masterClass(HMasterForTest.class) .numRegionServers(NUM_OF_RS).build()); From 20c454c19e5eb0f584d2e926d0824d22367285c6 Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Wed, 18 Jan 2023 16:32:51 +0000 Subject: [PATCH 4/5] addressing latest suggestions by Duo --- .../master/assignment/AssignmentManager.java | 44 +++++++++++++ .../TransitRegionStateProcedure.java | 64 ++++--------------- .../master/TestRetainAssignmentOnRestart.java | 5 +- 3 files changed, 60 insertions(+), 53 deletions(-) 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 4301a861bd71..54a12889c595 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 @@ -153,6 +153,25 @@ public class AssignmentManager { private static final int DEFAULT_RIT_STUCK_WARNING_THRESHOLD = 60 * 1000; public static final String UNEXPECTED_STATE_REGION = "Unexpected state for "; + public static final String FORCE_REGION_RETAINMENT = "hbase.master.scp.retain.assignment.force"; + + public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; + + /** The wait time in millis before checking again if the region's previous RS is back online */ + public static final String FORCE_REGION_RETAINMENT_WAIT = + "hbase.master.scp.retain.assignment.force.wait"; + + public static final int DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 100; + + /** + * The number of times to check if the region's previous RS is back online, before giving up and + * proceeding with assignment on a new RS + */ + public static final String FORCE_REGION_RETAINMENT_RETRIES = + "hbase.master.scp.retain.assignment.force.retries"; + + public static final long DEFAULT_FORCE_REGION_RETAINMENT_RETRIES = 600; + private final ProcedureEvent metaAssignEvent = new ProcedureEvent<>("meta assign"); private final ProcedureEvent metaLoadEvent = new ProcedureEvent<>("meta load"); @@ -201,6 +220,12 @@ public class AssignmentManager { private Thread assignThread; + private final boolean forceRegionRetainment; + + private final int forceRegionRetainmentWait; + + private final long forceRegionRetainmentRetries; + public AssignmentManager(MasterServices master, MasterRegion masterRegion) { this(master, masterRegion, new RegionStateStore(master, masterRegion)); } @@ -240,6 +265,13 @@ public AssignmentManager(MasterServices master, MasterRegion masterRegion) { } minVersionToMoveSysTables = conf.get(MIN_VERSION_MOVE_SYS_TABLES_CONFIG, DEFAULT_MIN_VERSION_MOVE_SYS_TABLES_CONFIG); + + forceRegionRetainment = + conf.getBoolean(FORCE_REGION_RETAINMENT, DEFAULT_FORCE_REGION_RETAINMENT); + forceRegionRetainmentWait = + conf.getInt(FORCE_REGION_RETAINMENT_WAIT, DEFAULT_FORCE_REGION_RETAINMENT_WAIT); + forceRegionRetainmentRetries = + conf.getLong(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); } private void mirrorMetaLocations() throws IOException, KeeperException { @@ -410,6 +442,18 @@ int getAssignMaxAttempts() { return assignMaxAttempts; } + public boolean isForceRegionRetainment() { + return forceRegionRetainment; + } + + public int getForceRegionRetainmentWait() { + return forceRegionRetainmentWait; + } + + public long getForceRegionRetainmentRetries() { + return forceRegionRetainmentRetries; + } + int getAssignRetryImmediatelyMaxAttempts() { return assignRetryImmediatelyMaxAttempts; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 7d196bc775c1..06b824a395d4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -20,6 +20,7 @@ import static org.apache.hadoop.hbase.io.hfile.CacheConfig.DEFAULT_EVICT_ON_CLOSE; import static org.apache.hadoop.hbase.io.hfile.CacheConfig.EVICT_BLOCKS_ON_CLOSE_KEY; import static org.apache.hadoop.hbase.master.LoadBalancer.BOGUS_SERVER_NAME; +import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT; import edu.umd.cs.findbugs.annotations.Nullable; import java.io.IOException; @@ -112,25 +113,6 @@ public class TransitRegionStateProcedure private static final Logger LOG = LoggerFactory.getLogger(TransitRegionStateProcedure.class); - public static final String FORCE_REGION_RETAINMENT = "hbase.master.scp.retain.assignment.force"; - - public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; - - /** The wait time in millis before checking again if the region's previous RS is back online */ - public static final String FORCE_REGION_RETAINMENT_WAIT = - "hbase.master.scp.retain.assignment.force.wait"; - - public static final int DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 100; - - /** - * The number of times to check if the region's previous RS is back online, before giving up and - * proceeding with assignment on a new RS - */ - public static final String FORCE_REGION_RETAINMENT_RETRIES = - "hbase.master.scp.retain.assignment.force.retries"; - - public static final long DEFAULT_FORCE_REGION_RETAINMENT_RETRIES = 600; - private TransitionType type; private RegionStateTransitionState initialState; @@ -150,14 +132,6 @@ public class TransitRegionStateProcedure private boolean isSplit; - private boolean forceRegionRetainment; - - private ServerManager serverManager; - - private int forceRegionRetainmentWait; - - private long forceRegionRetainmentRetries; - private long retries; public TransitRegionStateProcedure() { @@ -197,23 +171,6 @@ protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, } evictCache = env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE); - - readConfigs(env); - } - - private void readConfigs(MasterProcedureEnv env) { - forceRegionRetainment = env.getMasterConfiguration().getBoolean(FORCE_REGION_RETAINMENT, - DEFAULT_FORCE_REGION_RETAINMENT); - forceRegionRetainmentWait = env.getMasterConfiguration().getInt(FORCE_REGION_RETAINMENT_WAIT, - DEFAULT_FORCE_REGION_RETAINMENT_WAIT); - forceRegionRetainmentRetries = env.getMasterConfiguration() - .getLong(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); - serverManager = env.getMasterServices().getServerManager(); - } - - @Override - protected void afterReplay(MasterProcedureEnv env) { - readConfigs(env); } protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, @@ -239,23 +196,25 @@ protected boolean waitInitialized(MasterProcedureEnv env) { return am.waitMetaLoaded(this) || am.waitMetaAssigned(this, getRegion()); } - private void checkAndWaitForOriginalServer(ServerName lastHost) + private void checkAndWaitForOriginalServer(MasterProcedureEnv env, ServerName lastHost) throws ProcedureSuspendedException { + ServerManager serverManager = env.getMasterServices().getServerManager(); ServerName newNameForServer = serverManager.findServerWithSameHostnamePortWithLock(lastHost); boolean isOnline = serverManager.createDestinationServersList().contains(newNameForServer); - if (!isOnline && retries < forceRegionRetainmentRetries) { + + if (!isOnline && retries < env.getAssignmentManager().getForceRegionRetainmentRetries()) { retries++; LOG.info("Suspending the TRSP PID={} because {} is true and previous host {} " + "for region is not yet online.", this.getProcId(), FORCE_REGION_RETAINMENT, lastHost); - setTimeout(forceRegionRetainmentWait); + setTimeout(env.getAssignmentManager().getForceRegionRetainmentWait()); setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT); throw new ProcedureSuspendedException(); } LOG.info( "{} is true. TRSP PID={} waited {}ms for host {} to come back online. " + "Did host come back online? {}", - FORCE_REGION_RETAINMENT, this.getProcId(), (retries * forceRegionRetainmentWait), lastHost, - isOnline); + FORCE_REGION_RETAINMENT, this.getProcId(), + (retries * env.getAssignmentManager().getForceRegionRetainmentWait()), lastHost, isOnline); } private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) @@ -274,10 +233,13 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) regionNode.getRegionInfo().getEncodedName()); regionNode.setRegionLocation(regionNode.getLastHost()); } - if (regionNode.getRegionLocation() != null && forceRegionRetainment) { + if ( + regionNode.getRegionLocation() != null + && env.getAssignmentManager().isForceRegionRetainment() + ) { LOG.warn("{} is set to true. This may delay regions re-assignment " + "upon RegionServers crashes or restarts.", FORCE_REGION_RETAINMENT); - checkAndWaitForOriginalServer(regionNode.getRegionLocation()); + checkAndWaitForOriginalServer(env, regionNode.getRegionLocation()); } } LOG.info("Starting {}; {}; forceNewPlan={}, retain={}", this, regionNode.toShortString(), diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java index 52c2aea884ec..f45a3bddb394 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java @@ -17,8 +17,8 @@ */ package org.apache.hadoop.hbase.master; -import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT; -import static org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure.FORCE_REGION_RETAINMENT_WAIT; +import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT; +import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT_WAIT; import static org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure.MASTER_SCP_RETAIN_ASSIGNMENT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -192,6 +192,7 @@ public void testRetainAssignmentOnSingleRSRestart() throws Exception { cluster.stopMaster(0); cluster.waitForMasterToStop(master.getServerName(), 5000); cluster.stopRegionServer(deadRS); + cluster.waitForRegionServerToStop(deadRS, 5000); LOG.info("\n\nSleeping a bit"); Thread.sleep(2000); From b5d21a06fa69cb96ec43aa54f3018cd1244fda1b Mon Sep 17 00:00:00 2001 From: Wellington Ramos Chevreuil Date: Fri, 20 Jan 2023 17:26:14 +0000 Subject: [PATCH 5/5] Addressing latest review comments --- .../master/assignment/AssignmentManager.java | 24 +++++++------- .../TransitRegionStateProcedure.java | 33 ++++++++++++++----- .../master/TestRetainAssignmentOnRestart.java | 4 +-- 3 files changed, 39 insertions(+), 22 deletions(-) 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 54a12889c595..114f6d1e4950 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 @@ -158,10 +158,10 @@ public class AssignmentManager { public static final boolean DEFAULT_FORCE_REGION_RETAINMENT = false; /** The wait time in millis before checking again if the region's previous RS is back online */ - public static final String FORCE_REGION_RETAINMENT_WAIT = - "hbase.master.scp.retain.assignment.force.wait"; + public static final String FORCE_REGION_RETAINMENT_WAIT_INTERVAL = + "hbase.master.scp.retain.assignment.force.wait-interval"; - public static final int DEFAULT_FORCE_REGION_RETAINMENT_WAIT = 100; + public static final long DEFAULT_FORCE_REGION_RETAINMENT_WAIT_INTERVAL = 50; /** * The number of times to check if the region's previous RS is back online, before giving up and @@ -170,7 +170,7 @@ public class AssignmentManager { public static final String FORCE_REGION_RETAINMENT_RETRIES = "hbase.master.scp.retain.assignment.force.retries"; - public static final long DEFAULT_FORCE_REGION_RETAINMENT_RETRIES = 600; + public static final int DEFAULT_FORCE_REGION_RETAINMENT_RETRIES = 600; private final ProcedureEvent metaAssignEvent = new ProcedureEvent<>("meta assign"); private final ProcedureEvent metaLoadEvent = new ProcedureEvent<>("meta load"); @@ -222,9 +222,9 @@ public class AssignmentManager { private final boolean forceRegionRetainment; - private final int forceRegionRetainmentWait; + private final long forceRegionRetainmentWaitInterval; - private final long forceRegionRetainmentRetries; + private final int forceRegionRetainmentRetries; public AssignmentManager(MasterServices master, MasterRegion masterRegion) { this(master, masterRegion, new RegionStateStore(master, masterRegion)); @@ -268,10 +268,10 @@ public AssignmentManager(MasterServices master, MasterRegion masterRegion) { forceRegionRetainment = conf.getBoolean(FORCE_REGION_RETAINMENT, DEFAULT_FORCE_REGION_RETAINMENT); - forceRegionRetainmentWait = - conf.getInt(FORCE_REGION_RETAINMENT_WAIT, DEFAULT_FORCE_REGION_RETAINMENT_WAIT); + forceRegionRetainmentWaitInterval = conf.getLong(FORCE_REGION_RETAINMENT_WAIT_INTERVAL, + DEFAULT_FORCE_REGION_RETAINMENT_WAIT_INTERVAL); forceRegionRetainmentRetries = - conf.getLong(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); + conf.getInt(FORCE_REGION_RETAINMENT_RETRIES, DEFAULT_FORCE_REGION_RETAINMENT_RETRIES); } private void mirrorMetaLocations() throws IOException, KeeperException { @@ -446,11 +446,11 @@ public boolean isForceRegionRetainment() { return forceRegionRetainment; } - public int getForceRegionRetainmentWait() { - return forceRegionRetainmentWait; + public long getForceRegionRetainmentWaitInterval() { + return forceRegionRetainmentWaitInterval; } - public long getForceRegionRetainmentRetries() { + public int getForceRegionRetainmentRetries() { return forceRegionRetainmentRetries; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java index 06b824a395d4..81397915647d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java @@ -24,6 +24,7 @@ import edu.umd.cs.findbugs.annotations.Nullable; import java.io.IOException; +import java.util.concurrent.TimeUnit; import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; @@ -132,7 +133,9 @@ public class TransitRegionStateProcedure private boolean isSplit; - private long retries; + private RetryCounter forceRetainmentRetryCounter; + + private long forceRetainmentTotalWait; public TransitRegionStateProcedure() { } @@ -171,6 +174,16 @@ protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, } evictCache = env.getMasterConfiguration().getBoolean(EVICT_BLOCKS_ON_CLOSE_KEY, DEFAULT_EVICT_ON_CLOSE); + initForceRetainmentRetryCounter(env); + } + + private void initForceRetainmentRetryCounter(MasterProcedureEnv env) { + if (env.getAssignmentManager().isForceRegionRetainment()) { + forceRetainmentRetryCounter = + new RetryCounter(env.getAssignmentManager().getForceRegionRetainmentRetries(), + env.getAssignmentManager().getForceRegionRetainmentWaitInterval(), TimeUnit.MILLISECONDS); + forceRetainmentTotalWait = 0; + } } protected TransitRegionStateProcedure(MasterProcedureEnv env, RegionInfo hri, @@ -202,19 +215,23 @@ private void checkAndWaitForOriginalServer(MasterProcedureEnv env, ServerName la ServerName newNameForServer = serverManager.findServerWithSameHostnamePortWithLock(lastHost); boolean isOnline = serverManager.createDestinationServersList().contains(newNameForServer); - if (!isOnline && retries < env.getAssignmentManager().getForceRegionRetainmentRetries()) { - retries++; - LOG.info("Suspending the TRSP PID={} because {} is true and previous host {} " - + "for region is not yet online.", this.getProcId(), FORCE_REGION_RETAINMENT, lastHost); - setTimeout(env.getAssignmentManager().getForceRegionRetainmentWait()); + if (!isOnline && forceRetainmentRetryCounter.shouldRetry()) { + int backoff = + Math.toIntExact(forceRetainmentRetryCounter.getBackoffTimeAndIncrementAttempts()); + forceRetainmentTotalWait += backoff; + LOG.info( + "Suspending the TRSP PID={} for {}ms because {} is true and previous host {} " + + "for region is not yet online.", + this.getProcId(), backoff, FORCE_REGION_RETAINMENT, lastHost); + setTimeout(backoff); setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT); throw new ProcedureSuspendedException(); } LOG.info( "{} is true. TRSP PID={} waited {}ms for host {} to come back online. " + "Did host come back online? {}", - FORCE_REGION_RETAINMENT, this.getProcId(), - (retries * env.getAssignmentManager().getForceRegionRetainmentWait()), lastHost, isOnline); + FORCE_REGION_RETAINMENT, this.getProcId(), forceRetainmentTotalWait, lastHost, isOnline); + initForceRetainmentRetryCounter(env); } private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java index f45a3bddb394..2767bb106f43 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRetainAssignmentOnRestart.java @@ -18,7 +18,7 @@ package org.apache.hadoop.hbase.master; import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT; -import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT_WAIT; +import static org.apache.hadoop.hbase.master.assignment.AssignmentManager.FORCE_REGION_RETAINMENT_WAIT_INTERVAL; import static org.apache.hadoop.hbase.master.procedure.ServerCrashProcedure.MASTER_SCP_RETAIN_ASSIGNMENT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotEquals; @@ -237,7 +237,7 @@ public void testRetainAssignmentOnSingleRSRestart() throws Exception { @Test public void testForceRetainAssignment() throws Exception { UTIL.getConfiguration().setBoolean(FORCE_REGION_RETAINMENT, true); - UTIL.getConfiguration().setLong(FORCE_REGION_RETAINMENT_WAIT, 100); + UTIL.getConfiguration().setLong(FORCE_REGION_RETAINMENT_WAIT_INTERVAL, 50); setupCluster(); HMaster master = UTIL.getMiniHBaseCluster().getMaster(); SingleProcessHBaseCluster cluster = UTIL.getHBaseCluster();