From c4e3c7ee65dbf57e8bb566081524d33679d002c0 Mon Sep 17 00:00:00 2001 From: Phil Yang Date: Mon, 19 Jun 2017 15:49:42 +0800 Subject: [PATCH] HBASE-17931 Assign system tables to servers with highest version --- .../org/apache/hadoop/hbase/util/VersionInfo.java | 40 +++++++++++++++ .../apache/hadoop/hbase/util/TestVersionInfo.java | 35 +++++++++++++ .../hadoop/hbase/master/AssignmentManager.java | 57 +++++++++++++++++++++- .../org/apache/hadoop/hbase/master/HMaster.java | 20 ++++++-- .../apache/hadoop/hbase/master/MasterServices.java | 3 ++ .../apache/hadoop/hbase/master/ServerManager.java | 41 ++++++++++++++-- .../hbase/zookeeper/RegionServerTracker.java | 24 +++++++-- .../hbase/master/MockNoopMasterServices.java | 5 ++ .../hadoop/hbase/master/TestCatalogJanitor.java | 5 ++ 9 files changed, 218 insertions(+), 12 deletions(-) create mode 100644 hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestVersionInfo.java diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/VersionInfo.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/VersionInfo.java index 2b1fce7396..8b64de5e6b 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/VersionInfo.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/VersionInfo.java @@ -110,6 +110,46 @@ public class VersionInfo { } } + public static int compareVersion(String v1, String v2) { + //fast compare equals first + if (v1.equals(v2)) { + return 0; + } + + String s1[] = v1.split("\\.|-");//1.2.3-hotfix -> [1, 2, 3, hotfix] + String s2[] = v2.split("\\.|-"); + int index = 0; + while (index < s1.length && index < s2.length) { + int va = 10000, vb = 10000; + try { + va = Integer.parseInt(s1[index]); + } catch (Exception ingore) { + } + try { + vb = Integer.parseInt(s2[index]); + } catch (Exception ingore) { + } + if (va != vb) { + return va - vb; + } + if (va == 10000) { + // compare as String + int c = s1[index].compareTo(s2[index]); + if (c != 0) { + return c; + } + } + index++; + } + if (index < s1.length) { + // s1 is longer + return 1; + } + //s2 is longer + return -1; + } + + public static void main(String[] args) { writeTo(System.out); } diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestVersionInfo.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestVersionInfo.java new file mode 100644 index 0000000000..896c5241ec --- /dev/null +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/util/TestVersionInfo.java @@ -0,0 +1,35 @@ +/** + * 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.util; + +import org.apache.hadoop.hbase.testclassification.SmallTests; +import static org.junit.Assert.assertTrue; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category(SmallTests.class) +public class TestVersionInfo { + + @Test + public void testCompareVersion() { + assertTrue(VersionInfo.compareVersion("1.0.0", "0.98.11") > 0); + assertTrue(VersionInfo.compareVersion("0.98.11", "1.0.1") < 0); + assertTrue(VersionInfo.compareVersion("2.0.0", "1.4.0") > 0); + assertTrue(VersionInfo.compareVersion("2.0.0", "2.0.0-SNAPSHOT") < 0); + } +} \ No newline at end of file diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java index 0a28967bc2..27875d3116 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java @@ -24,6 +24,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; @@ -104,6 +105,7 @@ import org.apache.hadoop.hbase.util.PairOfSameType; import org.apache.hadoop.hbase.util.RetryCounter; import org.apache.hadoop.hbase.util.Threads; import org.apache.hadoop.hbase.util.Triple; +import org.apache.hadoop.hbase.util.VersionInfo; import org.apache.hadoop.hbase.wal.DefaultWALProvider; import org.apache.hadoop.hbase.zookeeper.MetaTableLocator; import org.apache.hadoop.hbase.zookeeper.ZKAssign; @@ -2427,11 +2429,41 @@ public class AssignmentManager extends ZooKeeperListener { } /** + * Get a list of servers that this region can not assign to. + * For system table, we must assign them to a server with highest version. + * RS will report to master before register on zk, and only when RS have registered on zk we can + * know the version. So in fact we will never assign a system region to a RS without registering on zk. + */ + public List getSystemTableExcludeServers() { + List> serverList = new ArrayList<>(); + for (ServerName s : serverManager.getOnlineServersList()) { + serverList.add(new Pair<>(s, server.getRegionServerVersion(s))); + } + if (serverList.isEmpty()) { + return new ArrayList<>(); + } + String highestVersion = Collections.max(serverList, new Comparator>() { + @Override + public int compare(Pair o1, Pair o2) { + return VersionInfo.compareVersion(o1.getSecond(), o2.getSecond()); + } + }).getSecond(); + List res = new ArrayList<>(); + for (Pair pair : serverList) { + if (!pair.getSecond().equals(highestVersion)) { + res.add(pair.getFirst()); + } + } + return res; + } + + + /** * @param region the region to assign * @return Plan for passed region (If none currently, it creates one or * if no servers to assign, it returns null). */ - private RegionPlan getRegionPlan(final HRegionInfo region, + public RegionPlan getRegionPlan(final HRegionInfo region, final boolean forceNewPlan) throws HBaseIOException { return getRegionPlan(region, null, forceNewPlan); } @@ -2449,8 +2481,15 @@ public class AssignmentManager extends ZooKeeperListener { final ServerName serverToExclude, final boolean forceNewPlan) throws HBaseIOException { // Pickup existing plan or make a new one final String encodedName = region.getEncodedName(); + List exclude = new ArrayList<>(); + if (region.isSystemTable()) { + exclude.addAll(getSystemTableExcludeServers()); + } + if (serverToExclude !=null) { + exclude.add(serverToExclude); + } final List destServers = - serverManager.createDestinationServersList(serverToExclude); + serverManager.createDestinationServersList(exclude); if (destServers.isEmpty()){ LOG.warn("Can't move " + encodedName + @@ -3471,6 +3510,20 @@ public class AssignmentManager extends ZooKeeperListener { return isCarryingRegion(serverName, metaHri); } + public List getCarryingSystemTables(ServerName serverName) throws IOException { + Set regions = this.getRegionStates().getServerRegions(serverName); + if (regions == null) { + return new ArrayList<>(); + } + List list = new ArrayList<>(); + for (HRegionInfo region : regions) { + if (region.isSystemTable()) { + list.add(region); + } + } + return list; + } + /** * Check if the shutdown server carries the specific region. * We have a bunch of places that store region location 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 25c65048e1..d3f8b4581f 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 @@ -889,6 +889,9 @@ public class HMaster extends HRegionServer implements MasterServices, Server { LOG.info("Master has completed initialization"); configurationManager.registerObserver(this.balancer); + serverManager.checkIfShouldMoveSystemRegion(); + regionServerTracker.masterInited(); + // Set master as 'initialized'. setInitialized(true); @@ -1625,7 +1628,7 @@ public class HMaster extends HRegionServer implements MasterServices, Server { @VisibleForTesting // Public so can be accessed by tests. public void move(final byte[] encodedRegionName, - final byte[] destServerName) throws HBaseIOException { + byte[] destServerName) throws HBaseIOException { RegionState regionState = assignmentManager.getRegionStates(). getRegionState(Bytes.toString(encodedRegionName)); if (regionState == null) { @@ -1639,11 +1642,20 @@ public class HMaster extends HRegionServer implements MasterServices, Server { HRegionInfo hri = regionState.getRegion(); ServerName dest; + List exclude = hri.isSystemTable() ? assignmentManager.getSystemTableExcludeServers() + : new ArrayList(1); + if (destServerName != null && exclude.contains(ServerName.valueOf(Bytes.toString(destServerName)))) { + LOG.info( + Bytes.toString(encodedRegionName) + " can not move to " + Bytes.toString(destServerName) + + " because the server is in exclude list"); + destServerName = null; + } + if (destServerName == null || destServerName.length == 0) { LOG.info("Passed destination servername is null/empty so " + "choosing a server at random"); - final List destServers = this.serverManager.createDestinationServersList( - regionState.getServerName()); + exclude.add(regionState.getServerName()); + final List destServers = this.serverManager.createDestinationServersList(exclude); dest = balancer.randomAssignment(hri, destServers); if (dest == null) { LOG.debug("Unable to determine a plan to assign " + hri); @@ -2501,7 +2513,7 @@ public class HMaster extends HRegionServer implements MasterServices, Server { if (info != null && info.hasVersionInfo()) { return info.getVersionInfo().getVersion(); } - return "Unknown"; + return "0.0.0"; //Lowest version to prevent move system region to unknown version RS. } /** 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 3fab2cc047..d74ca815c5 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 @@ -21,6 +21,7 @@ package org.apache.hadoop.hbase.master; import java.io.IOException; import java.util.List; +import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HRegionInfo; @@ -406,4 +407,6 @@ public interface MasterServices extends Server { * @throws IOException */ public long getLastMajorCompactionTimestampForRegion(byte[] regionName) throws IOException; + + public String getRegionServerVersion(final ServerName sn); } 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 71d03ce0cb..97bed5d59a 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 @@ -80,6 +80,7 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.RetryCounter; import org.apache.hadoop.hbase.util.RetryCounterFactory; +import org.apache.hadoop.hbase.util.Threads; import org.apache.hadoop.hbase.util.Triple; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZooKeeperWatcher; @@ -601,6 +602,40 @@ public class ServerManager { return ZKUtil.listChildrenNoWatch(zkw, zkw.rsZNode); } + public synchronized void checkIfShouldMoveSystemRegion() throws IOException { + // RS register on ZK after reports startup on master + List regionsShouldMove = new ArrayList<>(); + for (ServerName server : services.getAssignmentManager().getSystemTableExcludeServers()) { + while (true) { + try { + regionsShouldMove.addAll(services.getAssignmentManager().getCarryingSystemTables(server)); + break; + } catch (IOException e) { + LOG.warn("get system table assignment failed, will retry " + e.getMessage()); + Threads.sleep(2000); + } + } + + } + if (!regionsShouldMove.isEmpty()) { + AssignmentManager am = services.getAssignmentManager(); + List plans = new ArrayList<>(); + for (HRegionInfo regionInfo : regionsShouldMove) { + RegionPlan plan = am.getRegionPlan(regionInfo, true); + if (regionInfo.isMetaRegion()) { + // Must move meta region first. + am.balance(plan); + } else { + plans.add(plan); + } + } + for (RegionPlan plan : plans) { + am.balance(plan); + } + } + + } + /* * Expire the passed server. Add it to list of dead servers and queue a * shutdown processing. @@ -1241,11 +1276,11 @@ public class ServerManager { * the draining or dying servers. * @param serverToExclude can be null if there is no server to exclude */ - public List createDestinationServersList(final ServerName serverToExclude){ + public List createDestinationServersList(final List serversToExclude){ final List destServers = getOnlineServersList(); - if (serverToExclude != null){ - destServers.remove(serverToExclude); + if (serversToExclude != null){ + destServers.removeAll(serversToExclude); } // Loop through the draining server list and remove them from the server list diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java index 365010ff2e..05822aeaa0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/RegionServerTracker.java @@ -52,6 +52,7 @@ public class RegionServerTracker extends ZooKeeperListener { new TreeMap(); private ServerManager serverManager; private Server server; + private volatile boolean masterInited = false; public RegionServerTracker(ZooKeeperWatcher watcher, Server server, ServerManager serverManager) { @@ -72,10 +73,10 @@ public class RegionServerTracker extends ZooKeeperListener { watcher.registerListener(this); List servers = ZKUtil.listChildrenAndWatchThem(watcher, watcher.rsZNode); - add(servers); + refresh(servers); } - private void add(final List servers) throws IOException { + private void refresh(final List servers) throws IOException { synchronized(this.regionServers) { this.regionServers.clear(); for (String n: servers) { @@ -103,6 +104,23 @@ public class RegionServerTracker extends ZooKeeperListener { } } } + + if (masterInited) { + new Thread(new Runnable() { + @Override + public void run() { + try { + serverManager.checkIfShouldMoveSystemRegion(); + } catch (IOException e) { + LOG.error(e); + } + } + }).start(); + } + } + + public void masterInited() { + masterInited = true; } private void remove(final ServerName sn) { @@ -135,7 +153,7 @@ public class RegionServerTracker extends ZooKeeperListener { try { List servers = ZKUtil.listChildrenAndWatchThem(watcher, watcher.rsZNode); - add(servers); + refresh(servers); } catch (IOException e) { server.abort("Unexpected zk exception getting RS nodes", e); } catch (KeeperException e) { 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 a172de5cb0..122d65490d 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 @@ -240,6 +240,11 @@ public class MockNoopMasterServices implements MasterServices, Server { } @Override + public String getRegionServerVersion(ServerName sn) { + return null; + } + + @Override public Configuration getConfiguration() { return null; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java index 01a2124a9e..53ca59d260 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java @@ -558,6 +558,11 @@ public class TestCatalogJanitor { // Auto-generated method stub return 0; } + + @Override + public String getRegionServerVersion(ServerName sn) { + return null; + } } @Test -- 2.11.0 (Apple Git-81)