You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2021/05/08 01:30:30 UTC
[hbase] branch branch-2.3 updated: HBASE-25774
ServerManager.getOnlineServer may miss some region servers when refreshing
state in some procedure implementations
This is an automated email from the ASF dual-hosted git repository.
apurtell pushed a commit to branch branch-2.3
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2.3 by this push:
new d3b5464 HBASE-25774 ServerManager.getOnlineServer may miss some region servers when refreshing state in some procedure implementations
d3b5464 is described below
commit d3b5464d38998f8344097380dfeb65a0717e110b
Author: Andrew Purtell <ap...@apache.org>
AuthorDate: Fri May 7 17:57:24 2021 -0700
HBASE-25774 ServerManager.getOnlineServer may miss some region servers when refreshing state in some procedure implementations
Revert "HBASE-25032 Wait for region server to become online before adding it to online servers in Master (#2771) (#3095)"
This reverts commit 475667837b6b04e4c0315575213160fe16a08e11.
---
.../apache/hadoop/hbase/master/ServerManager.java | 8 +++--
.../hadoop/hbase/regionserver/HRegionServer.java | 34 ++++++++++------------
.../org/apache/hadoop/hbase/MiniHBaseCluster.java | 3 +-
.../hbase/master/TestGetReplicationLoad.java | 3 +-
.../hadoop/hbase/master/TestMasterMetrics.java | 6 ++++
.../TestCompactionInDeadRegionServer.java | 3 +-
.../regionserver/TestShutdownWhileWALBroken.java | 3 +-
7 files changed, 32 insertions(+), 28 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 ef967fc..9dc3957 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
@@ -178,8 +178,7 @@ public class ServerManager {
}
/**
- * Let the server manager know a regionserver is requesting configurations.
- * Regionserver will not be added here, but in its first report.
+ * Let the server manager know a new regionserver has come online
* @param request the startup request
* @param versionNumber the version number of the new regionserver
* @param version the version of the new regionserver, could contain strings like "SNAPSHOT"
@@ -202,6 +201,10 @@ public class ServerManager {
ServerName sn = ServerName.valueOf(hostname, request.getPort(), request.getServerStartCode());
checkClockSkew(sn, request.getServerCurrentTime());
checkIsDead(sn, "STARTUP");
+ if (!checkAndRecordNewServer(sn, ServerMetricsBuilder.of(sn, versionNumber, version))) {
+ LOG.warn(
+ "THIS SHOULD NOT HAPPEN, RegionServerStartup" + " could not record the server: " + sn);
+ }
return sn;
}
@@ -253,6 +256,7 @@ public class ServerManager {
if (null == this.onlineServers.replace(sn, sl)) {
// Already have this host+port combo and its just different start code?
// Just let the server in. Presume master joining a running cluster.
+ // recordNewServer is what happens at the end of reportServerStartup.
// The only thing we are skipping is passing back to the regionserver
// the ServerName to use. Here we presume a master has already done
// that so we'll press on with whatever it gave us for ServerName.
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 a82a56c..0560c25 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
@@ -975,9 +975,10 @@ public class HRegionServer extends Thread implements
// node was created, in case any coprocessors want to use ZooKeeper
this.rsHost = new RegionServerCoprocessorHost(this, this.conf);
- // Get configurations from the Master. Break if server is stopped or
- // the clusterup flag is down or hdfs went wacky. Then start up all Services.
- // Use RetryCounter to get backoff in case Master is struggling to come up.
+ // Try and register with the Master; tell it we are here. Break if server is stopped or
+ // the clusterup flag is down or hdfs went wacky. Once registered successfully, go ahead and
+ // start up all Services. Use RetryCounter to get backoff in case Master is struggling to
+ // come up.
LOG.debug("About to register with Master.");
RetryCounterFactory rcf =
new RetryCounterFactory(Integer.MAX_VALUE, this.sleeper.getPeriod(), 1000 * 60 * 5);
@@ -1010,7 +1011,7 @@ public class HRegionServer extends Thread implements
}
}
- // Run mode.
+ // We registered with the Master. Go into run mode.
long lastMsg = System.currentTimeMillis();
long oldRequestCount = -1;
// The main run loop.
@@ -1044,14 +1045,7 @@ public class HRegionServer extends Thread implements
}
long now = System.currentTimeMillis();
if ((now - lastMsg) >= msgInterval) {
- // Register with the Master now that our setup is complete.
- if (tryRegionServerReport(lastMsg, now) && !online.get()) {
- // Wake up anyone waiting for this server to online
- synchronized (online) {
- online.set(true);
- online.notifyAll();
- }
- }
+ tryRegionServerReport(lastMsg, now);
lastMsg = System.currentTimeMillis();
}
if (!isStopped() && !isAborted()) {
@@ -1221,12 +1215,12 @@ public class HRegionServer extends Thread implements
}
@VisibleForTesting
- protected boolean tryRegionServerReport(long reportStartTime, long reportEndTime)
+ protected void tryRegionServerReport(long reportStartTime, long reportEndTime)
throws IOException {
RegionServerStatusService.BlockingInterface rss = rssStub;
if (rss == null) {
// the current server could be stopping.
- return false;
+ return;
}
ClusterStatusProtos.ServerLoad sl = buildServerLoad(reportStartTime, reportEndTime);
try {
@@ -1246,9 +1240,7 @@ public class HRegionServer extends Thread implements
// Couldn't connect to the master, get location from zk and reconnect
// Method blocks until new master is found or we are stopped
createRegionServerStatusStub(true);
- return false;
}
- return true;
}
/**
@@ -1611,6 +1603,11 @@ public class HRegionServer extends Thread implements
", sessionid=0x" +
Long.toHexString(this.zooKeeper.getRecoverableZooKeeper().getSessionId()));
+ // Wake up anyone waiting for this server to online
+ synchronized (online) {
+ online.set(true);
+ online.notifyAll();
+ }
} catch (Throwable e) {
stop("Failed initialization");
throw convertThrowableToIOE(cleanup(e, "Failed init"),
@@ -2729,9 +2726,10 @@ public class HRegionServer extends Thread implements
}
/*
- * Run initialization using parameters passed us by the master.
+ * Let the master know we're here Run initialization using parameters passed
+ * us by the master.
* @return A Map of key/value configurations we got from the Master else
- * null if we failed during report.
+ * null if we failed to register.
* @throws IOException
*/
private RegionServerStartupResponse reportForDuty() throws IOException {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
index 89d1968..f795eef 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/MiniHBaseCluster.java
@@ -449,9 +449,8 @@ public class MiniHBaseCluster extends HBaseCluster {
ServerName rsServerName = t.getRegionServer().getServerName();
long start = System.currentTimeMillis();
- ClusterStatus clusterStatus;
+ ClusterStatus clusterStatus = getClusterStatus();
while ((System.currentTimeMillis() - start) < timeout) {
- clusterStatus = getClusterStatus();
if (clusterStatus != null && clusterStatus.getServers().contains(rsServerName)) {
return t;
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestGetReplicationLoad.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestGetReplicationLoad.java
index 1fcd646..ac9ae39 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestGetReplicationLoad.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestGetReplicationLoad.java
@@ -58,9 +58,8 @@ public class TestGetReplicationLoad {
}
@Override
- protected boolean tryRegionServerReport(long reportStartTime, long reportEndTime) {
+ protected void tryRegionServerReport(long reportStartTime, long reportEndTime) {
// do nothing
- return true;
}
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
index 4e9bf10..5116933 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java
@@ -60,6 +60,12 @@ public class TestMasterMetrics {
public MyMaster(Configuration conf) throws IOException, KeeperException, InterruptedException {
super(conf);
}
+ @Override
+ protected void tryRegionServerReport(
+ long reportStartTime, long reportEndTime) {
+ // do nothing
+ }
+
}
@BeforeClass
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionInDeadRegionServer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionInDeadRegionServer.java
index 9b5c523..78042cc 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionInDeadRegionServer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompactionInDeadRegionServer.java
@@ -84,14 +84,13 @@ public class TestCompactionInDeadRegionServer {
}
@Override
- protected boolean tryRegionServerReport(long reportStartTime, long reportEndTime)
+ protected void tryRegionServerReport(long reportStartTime, long reportEndTime)
throws IOException {
try {
super.tryRegionServerReport(reportStartTime, reportEndTime);
} catch (YouAreDeadException e) {
// ignore, do not abort
}
- return true;
}
}
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java
index ffc3a19..30c954b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestShutdownWhileWALBroken.java
@@ -85,14 +85,13 @@ public class TestShutdownWhileWALBroken {
}
@Override
- protected boolean tryRegionServerReport(long reportStartTime, long reportEndTime)
+ protected void tryRegionServerReport(long reportStartTime, long reportEndTime)
throws IOException {
try {
super.tryRegionServerReport(reportStartTime, reportEndTime);
} catch (YouAreDeadException e) {
LOG.info("Caught YouAreDeadException, ignore", e);
}
- return true;
}
@Override