From 204a4539c4a3dbab45dd26ae79bd9650b123101b Mon Sep 17 00:00:00 2001 From: zhangduo Date: Sun, 14 Jan 2018 20:45:31 +0800 Subject: [PATCH] HBASE-19793 Minor improvements on Master/RS startup --- .../apache/hadoop/hbase/master/HMaster.java | 13 +- .../hadoop/hbase/master/MasterFileSystem.java | 7 +- .../hbase/regionserver/HRegionServer.java | 130 ++++++++---------- .../master/assignment/MockMasterServices.java | 2 +- 4 files changed, 74 insertions(+), 78 deletions(-) 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 971ff08add2..2683a6afc4f 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 @@ -53,6 +53,7 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServlet; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.apache.commons.lang3.StringUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.ClusterId; @@ -536,6 +537,11 @@ public class HMaster extends HRegionServer implements MasterServices { } } + @Override + protected String getUseThisHostnameInstead(Configuration conf) { + return conf.get(MASTER_HOSTNAME_KEY); + } + // Main run loop. Calls through to the regionserver run loop AFTER becoming active Master; will // block in here until then. @Override @@ -608,7 +614,8 @@ public class HMaster extends HRegionServer implements MasterServices { masterJettyServer.addConnector(connector); masterJettyServer.setStopAtShutdown(true); - final String redirectHostname = shouldUseThisHostnameInstead() ? useThisHostnameInstead : null; + final String redirectHostname = + StringUtils.isBlank(useThisHostnameInstead) ? null : useThisHostnameInstead; final RedirectServlet redirect = new RedirectServlet(infoServer, redirectHostname); final WebAppContext context = new WebAppContext(null, "/", null, null, null, null, WebAppContext.NO_SESSIONS); @@ -785,7 +792,7 @@ public class HMaster extends HRegionServer implements MasterServices { // TODO: Do this using Dependency Injection, using PicoContainer, Guice or Spring. // Initialize the chunkCreator initializeMemStoreChunkCreator(); - this.fileSystemManager = new MasterFileSystem(this); + this.fileSystemManager = new MasterFileSystem(conf); this.walManager = new MasterWalManager(this); // enable table descriptors cache @@ -803,7 +810,7 @@ public class HMaster extends HRegionServer implements MasterServices { ClusterId clusterId = fileSystemManager.getClusterId(); status.setStatus("Publishing Cluster ID " + clusterId + " in ZooKeeper"); ZKClusterId.setClusterId(this.zooKeeper, fileSystemManager.getClusterId()); - this.clusterId = ZKClusterId.readClusterIdZNode(this.zooKeeper); + this.clusterId = clusterId.toString(); this.serverManager = createServerManager(this); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index 8c2c9fdcdc6..a37fd4e6506 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -98,11 +98,8 @@ public class MasterFileSystem { private boolean isSecurityEnabled; - private final MasterServices services; - - public MasterFileSystem(MasterServices services) throws IOException { - this.conf = services.getConfiguration(); - this.services = services; + public MasterFileSystem(Configuration conf) throws IOException { + this.conf = conf; // Set filesystem to be that of this.rootdir else we get complaints about // mismatched filesystems if hbase.rootdir is hdfs and fs.defaultFS is // default localfs. Presumption is that rootdir is fully-qualified before 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 8e9170240a3..37ec5959218 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 @@ -28,6 +28,7 @@ import java.lang.reflect.Constructor; import java.net.BindException; import java.net.InetAddress; import java.net.InetSocketAddress; +import java.time.Duration; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -45,13 +46,12 @@ import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ConcurrentSkipListMap; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; import org.apache.commons.lang3.RandomUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.SystemUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileSystem; @@ -168,13 +168,13 @@ import org.apache.hadoop.hbase.zookeeper.ZNodePaths; import org.apache.hadoop.ipc.RemoteException; import org.apache.hadoop.metrics2.util.MBeans; import org.apache.hadoop.util.ReflectionUtils; -import org.apache.hadoop.util.StringUtils; import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.KeeperException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; +import org.apache.hbase.thirdparty.com.google.common.base.Throwables; import org.apache.hbase.thirdparty.com.google.common.collect.Maps; import org.apache.hbase.thirdparty.com.google.protobuf.BlockingRpcChannel; import org.apache.hbase.thirdparty.com.google.protobuf.RpcController; @@ -379,13 +379,13 @@ public class HRegionServer extends HasThread implements final AtomicBoolean online = new AtomicBoolean(false); // zookeeper connection and watcher - protected ZKWatcher zooKeeper; + protected final ZKWatcher zooKeeper; // master address tracker - private MasterAddressTracker masterAddressTracker; + private final MasterAddressTracker masterAddressTracker; // Cluster Status Tracker - protected ClusterStatusTracker clusterStatusTracker; + protected final ClusterStatusTracker clusterStatusTracker; // Log Splitting Worker private SplitLogWorker splitLogWorker; @@ -518,7 +518,6 @@ public class HRegionServer extends HasThread implements private final boolean masterless; static final String MASTERLESS_CONFIG_NAME = "hbase.masterless"; - /** * Starts a HRegionServer at the default location */ @@ -565,23 +564,10 @@ public class HRegionServer extends HasThread implements this.stopped = false; rpcServices = createRpcServices(); - if (this instanceof HMaster) { - useThisHostnameInstead = conf.get(MASTER_HOSTNAME_KEY); - } else { - useThisHostnameInstead = conf.get(RS_HOSTNAME_KEY); - if (conf.getBoolean(RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false)) { - if (shouldUseThisHostnameInstead()) { - String msg = RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " + RS_HOSTNAME_KEY + - " are mutually exclusive. Do not set " + RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + - " to true while " + RS_HOSTNAME_KEY + " is used"; - throw new IOException(msg); - } else { - useThisHostnameInstead = rpcServices.isa.getHostName(); - } - } - } - String hostName = shouldUseThisHostnameInstead() ? - this.useThisHostnameInstead : this.rpcServices.isa.getHostName(); + useThisHostnameInstead = getUseThisHostnameInstead(conf); + String hostName = + StringUtils.isBlank(useThisHostnameInstead) ? this.rpcServices.isa.getHostName() + : this.useThisHostnameInstead; serverName = ServerName.valueOf(hostName, this.rpcServices.isa.getPort(), this.startcode); rpcControllerFactory = RpcControllerFactory.instantiate(this.conf); @@ -617,7 +603,6 @@ public class HRegionServer extends HasThread implements // Open connection to zookeeper and set primary watcher zooKeeper = new ZKWatcher(conf, getProcessName() + ":" + rpcServices.isa.getPort(), this, canCreateBaseZNode()); - // If no master in cluster, skip trying to track one or look for a cluster status. if (!this.masterless) { this.csm = new ZkCoordinatedStateManager(this); @@ -627,7 +612,14 @@ public class HRegionServer extends HasThread implements clusterStatusTracker = new ClusterStatusTracker(zooKeeper, this); clusterStatusTracker.start(); + } else { + masterAddressTracker = null; + clusterStatusTracker = null; } + } else { + zooKeeper = null; + masterAddressTracker = null; + clusterStatusTracker = null; } // This violates 'no starting stuff in Constructor' but Master depends on the below chore // and executor being created and takes a different startup route. Lots of overlap between HRS @@ -646,6 +638,23 @@ public class HRegionServer extends HasThread implements } } + // HMaster should override this method to load the specific config for master + protected String getUseThisHostnameInstead(Configuration conf) throws IOException { + String hostname = conf.get(RS_HOSTNAME_KEY); + if (conf.getBoolean(RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY, false)) { + if (!StringUtils.isBlank(hostname)) { + String msg = RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + " and " + RS_HOSTNAME_KEY + + " are mutually exclusive. Do not set " + RS_HOSTNAME_DISABLE_MASTER_REVERSEDNS_KEY + + " to true while " + RS_HOSTNAME_KEY + " is used"; + throw new IOException(msg); + } else { + return rpcServices.isa.getHostName(); + } + } else { + return hostname; + } + } + /** * If running on Windows, do windows-specific setup. */ @@ -695,13 +704,6 @@ public class HRegionServer extends HasThread implements return null; } - /* - * Returns true if configured hostname should be used - */ - protected boolean shouldUseThisHostnameInstead() { - return useThisHostnameInstead != null && !useThisHostnameInstead.isEmpty(); - } - protected void login(UserProvider user, String host) throws IOException { user.login("hbase.regionserver.keytab.file", "hbase.regionserver.kerberos.principal", host); @@ -804,17 +806,14 @@ public class HRegionServer extends HasThread implements } /** - * All initialization needed before we go register with Master. - * Do bare minimum. Do bulk of initializations AFTER we've connected to the Master. + * All initialization needed before we go register with Master.
+ * Do bare minimum. Do bulk of initializations AFTER we've connected to the Master.
* In here we just put up the RpcServer, setup Connection, and ZooKeeper. - * - * @throws IOException - * @throws InterruptedException */ private void preRegistrationInitialization() { try { - setupClusterConnection(); initializeZooKeeper(); + setupClusterConnection(); // Setup RPC client for master communication this.rpcClient = RpcClientFactory.createClient(conf, clusterId, new InetSocketAddress( this.rpcServices.isa.getAddress(), 0), clusterConnection.getConnectionMetrics()); @@ -827,18 +826,18 @@ public class HRegionServer extends HasThread implements } /** - * Bring up connection to zk ensemble and then wait until a master for this - * cluster and then after that, wait until cluster 'up' flag has been set. - * This is the order in which master does things. + * Bring up connection to zk ensemble and then wait until a master for this cluster and then after + * that, wait until cluster 'up' flag has been set. This is the order in which master does things. + *

* Finally open long-living server short-circuit connection. - * @throws IOException - * @throws InterruptedException */ @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE", justification="cluster Id znode read would give us correct response") private void initializeZooKeeper() throws IOException, InterruptedException { // Nothing to do in here if no Master in the mix. - if (this.masterless) return; + if (this.masterless) { + return; + } // Create the master address tracker, register with zk, and start it. Then // block until a master is available. No point in starting up if no master @@ -849,17 +848,20 @@ public class HRegionServer extends HasThread implements // when ready. blockAndCheckIfStopped(this.clusterStatusTracker); - // Retrieve clusterId - // Since cluster status is now up - // ID should have already been set by HMaster - try { - clusterId = ZKClusterId.readClusterIdZNode(this.zooKeeper); - if (clusterId == null) { - this.abort("Cluster ID has not been set"); + // If we are HMaster then the cluster id should have already been set. + if (clusterId == null) { + // Retrieve clusterId + // Since cluster status is now up + // ID should have already been set by HMaster + try { + clusterId = ZKClusterId.readClusterIdZNode(this.zooKeeper); + if (clusterId == null) { + this.abort("Cluster ID has not been set"); + } + LOG.info("ClusterId : " + clusterId); + } catch (KeeperException e) { + this.abort("Failed to retrieve Cluster ID", e); } - LOG.info("ClusterId : "+clusterId); - } catch (KeeperException e) { - this.abort("Failed to retrieve Cluster ID",e); } // In case colocated master, wait here till it's active. @@ -881,16 +883,6 @@ public class HRegionServer extends HasThread implements } } - @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="RV_RETURN_VALUE_IGNORED", - justification="We don't care about the return") - private void doLatch(final CountDownLatch latch) throws InterruptedException { - if (latch != null) { - // Result is ignored intentionally but if I remove the below, findbugs complains (the - // above justification on this method doesn't seem to suppress it). - boolean result = latch.await(20, TimeUnit.SECONDS); - } - } - /** * Utilty method to wait indefinitely on a znode availability while checking * if the region server is shut down @@ -1461,14 +1453,14 @@ public class HRegionServer extends HasThread implements String hostnameFromMasterPOV = e.getValue(); this.serverName = ServerName.valueOf(hostnameFromMasterPOV, rpcServices.isa.getPort(), this.startcode); - if (shouldUseThisHostnameInstead() && + if (!StringUtils.isBlank(useThisHostnameInstead) && !hostnameFromMasterPOV.equals(useThisHostnameInstead)) { String msg = "Master passed us a different hostname to use; was=" + this.useThisHostnameInstead + ", but now=" + hostnameFromMasterPOV; LOG.error(msg); throw new IOException(msg); } - if (!shouldUseThisHostnameInstead() && + if (StringUtils.isBlank(useThisHostnameInstead) && !hostnameFromMasterPOV.equals(rpcServices.isa.getHostName())) { String msg = "Master passed us a different hostname to use; was=" + rpcServices.isa.getHostName() + ", but now=" + hostnameFromMasterPOV; @@ -1685,7 +1677,7 @@ public class HRegionServer extends HasThread implements CompactionChecker(final HRegionServer h, final int sleepTime, final Stoppable stopper) { super("CompactionChecker", stopper, sleepTime); this.instance = h; - LOG.info(this.getName() + " runs every " + StringUtils.formatTime(sleepTime)); + LOG.info(this.getName() + " runs every " + Duration.ofMillis(sleepTime)); /* MajorCompactPriority is configurable. * If not set, the compaction will use default priority. @@ -2375,7 +2367,7 @@ public class HRegionServer extends HasThread implements // Do our best to report our abort to the master, but this may not work try { if (cause != null) { - msg += "\nCause:\n" + StringUtils.stringifyException(cause); + msg += "\nCause:\n" + Throwables.getStackTraceAsString(cause); } // Report to the master but only if we have already registered with the master. if (rssStub != null && this.serverName != null) { @@ -2603,7 +2595,7 @@ public class HRegionServer extends HasThread implements long now = EnvironmentEdgeManager.currentTime(); int port = rpcServices.isa.getPort(); RegionServerStartupRequest.Builder request = RegionServerStartupRequest.newBuilder(); - if (shouldUseThisHostnameInstead()) { + if (!StringUtils.isBlank(useThisHostnameInstead)) { request.setUseThisHostnameInstead(useThisHostnameInstead); } request.setPort(port); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java index 45d5b08d663..83fafff970f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/assignment/MockMasterServices.java @@ -103,7 +103,7 @@ public class MockMasterServices extends MockNoopMasterServices { super(conf); this.regionsToRegionServers = regionsToRegionServers; Superusers.initialize(conf); - this.fileSystemManager = new MasterFileSystem(this); + this.fileSystemManager = new MasterFileSystem(conf); this.walManager = new MasterWalManager(this); // Mock an AM. this.assignmentManager = new AssignmentManager(this, new MockRegionStateStore(this)) {