From bf78246b4f8fc1ce37421c746a8209fe8991a0db Mon Sep 17 00:00:00 2001 From: Duo Zhang Date: Fri, 16 Apr 2021 09:50:24 +0800 Subject: [PATCH] HBASE-25775 Use a special balancer to deal with maintenance mode (#3161) Signed-off-by: Wellington Chevreuil --- .../apache/hadoop/hbase/master/HMaster.java | 9 +- .../master/balancer/BaseLoadBalancer.java | 51 +++---- .../balancer/MaintenanceLoadBalancer.java | 132 ++++++++++++++++++ .../hbase/master/TestMasterRepairMode.java | 44 +++--- 4 files changed, 189 insertions(+), 47 deletions(-) create mode 100644 hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java 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 fe4ada4cd5e..985ba128d73 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 @@ -103,6 +103,7 @@ import org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure; import org.apache.hadoop.hbase.master.balancer.BalancerChore; import org.apache.hadoop.hbase.master.balancer.ClusterStatusChore; import org.apache.hadoop.hbase.master.balancer.LoadBalancerFactory; +import org.apache.hadoop.hbase.master.balancer.MaintenanceLoadBalancer; import org.apache.hadoop.hbase.master.cleaner.DirScanPool; import org.apache.hadoop.hbase.master.cleaner.HFileCleaner; import org.apache.hadoop.hbase.master.cleaner.LogCleaner; @@ -677,7 +678,13 @@ public class HMaster extends HRegionServer implements MasterServices { * should have already been initialized along with {@link ServerManager}. */ private void initializeZKBasedSystemTrackers() - throws IOException, KeeperException, ReplicationException { + throws IOException, KeeperException, ReplicationException { + if (maintenanceMode) { + // in maintenance mode, always use MaintenanceLoadBalancer. + conf.unset(LoadBalancer.HBASE_RSGROUP_LOADBALANCER_CLASS); + conf.setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, MaintenanceLoadBalancer.class, + LoadBalancer.class); + } this.balancer = new RSGroupBasedLoadBalancer(); this.balancer.setConf(conf); this.loadBalancerTracker = new LoadBalancerTracker(zooKeeper, this); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java index b411636e537..99873dd3176 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java @@ -35,6 +35,7 @@ import java.util.NavigableMap; import java.util.Random; import java.util.Set; import java.util.TreeMap; +import java.util.concurrent.ThreadLocalRandom; import java.util.function.Predicate; import java.util.stream.Collectors; import org.apache.commons.lang3.NotImplementedException; @@ -1101,7 +1102,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { protected float overallSlop; protected Configuration config = HBaseConfiguration.create(); protected RackManager rackManager; - private static final Random RANDOM = new Random(System.currentTimeMillis()); private static final Logger LOG = LoggerFactory.getLogger(BaseLoadBalancer.class); protected MetricsBalancer metricsBalancer = null; protected ClusterMetrics clusterStatus = null; @@ -1115,17 +1115,21 @@ public abstract class BaseLoadBalancer implements LoadBalancer { @Deprecated protected boolean onlySystemTablesOnMaster; - protected boolean maintenanceMode; - @Override public void setConf(Configuration conf) { this.config = conf; setSlop(conf); - if (slop < 0) slop = 0; - else if (slop > 1) slop = 1; + if (slop < 0) { + slop = 0; + } else if (slop > 1) { + slop = 1; + } - if (overallSlop < 0) overallSlop = 0; - else if (overallSlop > 1) overallSlop = 1; + if (overallSlop < 0) { + overallSlop = 0; + } else if (overallSlop > 1) { + overallSlop = 1; + } this.onlySystemTablesOnMaster = LoadBalancer.isSystemTablesOnlyOnMaster(this.config); @@ -1135,8 +1139,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { } this.isByTable = conf.getBoolean(HConstants.HBASE_MASTER_LOADBALANCE_BYTABLE, isByTable); // Print out base configs. Don't print overallSlop since it for simple balancer exclusively. - LOG.info("slop={}, systemTablesOnMaster={}", - this.slop, this.onlySystemTablesOnMaster); + LOG.info("slop={}, systemTablesOnMaster={}", this.slop, this.onlySystemTablesOnMaster); } protected void setSlop(Configuration conf) { @@ -1153,8 +1156,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { */ @Deprecated public boolean shouldBeOnMaster(RegionInfo region) { - return (this.maintenanceMode || this.onlySystemTablesOnMaster) - && region.getTable().isSystemTable(); + return this.onlySystemTablesOnMaster && region.getTable().isSystemTable(); } /** @@ -1221,7 +1223,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { protected Map> assignMasterSystemRegions( Collection regions, List servers) { Map> assignments = new TreeMap<>(); - if (this.maintenanceMode || this.onlySystemTablesOnMaster) { + if (this.onlySystemTablesOnMaster) { if (masterServerName != null && servers.contains(masterServerName)) { assignments.put(masterServerName, new ArrayList<>()); for (RegionInfo region : regions) { @@ -1255,9 +1257,6 @@ public abstract class BaseLoadBalancer implements LoadBalancer { if (useRegionFinder) { this.regionFinder.setClusterInfoProvider(new MasterClusterInfoProvider(services)); } - if (this.services.isInMaintenanceMode()) { - this.maintenanceMode = true; - } } @Override @@ -1372,7 +1371,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { /** * only need assign system table */ - if (this.maintenanceMode || regions.isEmpty()) { + if (regions.isEmpty()) { return assignments; } @@ -1512,7 +1511,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { regions = regions.entrySet().stream().filter(e -> !masterRegions.contains(e.getKey())) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); } - if (this.maintenanceMode || regions.isEmpty()) { + if (regions.isEmpty()) { return assignments; } @@ -1659,8 +1658,9 @@ public abstract class BaseLoadBalancer implements LoadBalancer { final int maxIterations = numServers * 4; int iterations = 0; List usedSNs = new ArrayList<>(servers.size()); + Random rand = ThreadLocalRandom.current(); do { - int i = RANDOM.nextInt(numServers); + int i = rand.nextInt(numServers); sn = servers.get(i); if (!usedSNs.contains(sn)) { usedSNs.add(sn); @@ -1690,13 +1690,14 @@ public abstract class BaseLoadBalancer implements LoadBalancer { */ private void roundRobinAssignment(Cluster cluster, List regions, List servers, Map> assignments) { + Random rand = ThreadLocalRandom.current(); List unassignedRegions = new ArrayList<>(); int numServers = servers.size(); int numRegions = regions.size(); int max = (int) Math.ceil((float) numRegions / numServers); int serverIdx = 0; if (numServers > 1) { - serverIdx = RANDOM.nextInt(numServers); + serverIdx = rand.nextInt(numServers); } int regionIdx = 0; for (int j = 0; j < numServers; j++) { @@ -1718,17 +1719,17 @@ public abstract class BaseLoadBalancer implements LoadBalancer { List lastFewRegions = new ArrayList<>(); // assign the remaining by going through the list and try to assign to servers one-by-one - serverIdx = RANDOM.nextInt(numServers); - OUTER : for (RegionInfo region : unassignedRegions) { + serverIdx = rand.nextInt(numServers); + for (RegionInfo region : unassignedRegions) { boolean assigned = false; - INNER : for (int j = 0; j < numServers; j++) { // try all servers one by one + for (int j = 0; j < numServers; j++) { // try all servers one by one ServerName server = servers.get((j + serverIdx) % numServers); if (cluster.wouldLowerAvailability(region, server)) { - continue INNER; + continue; } else { assignments.computeIfAbsent(server, k -> new ArrayList<>()).add(region); cluster.doAssignRegion(region, server); - serverIdx = (j + serverIdx + 1) % numServers; //remain from next server + serverIdx = (j + serverIdx + 1) % numServers; // remain from next server assigned = true; break; } @@ -1740,7 +1741,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer { // just sprinkle the rest of the regions on random regionservers. The balanceCluster will // make it optimal later. we can end up with this if numReplicas > numServers. for (RegionInfo region : lastFewRegions) { - int i = RANDOM.nextInt(numServers); + int i = rand.nextInt(numServers); ServerName server = servers.get(i); assignments.computeIfAbsent(server, k -> new ArrayList<>()).add(region); cluster.doAssignRegion(region, server); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java new file mode 100644 index 00000000000..12d4cc0c8a9 --- /dev/null +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/MaintenanceLoadBalancer.java @@ -0,0 +1,132 @@ +/** + * 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.master.balancer; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.conf.Configured; +import org.apache.hadoop.hbase.ClusterMetrics; +import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.RegionInfo; +import org.apache.hadoop.hbase.master.LoadBalancer; +import org.apache.hadoop.hbase.master.MasterServices; +import org.apache.hadoop.hbase.master.RegionPlan; +import org.apache.yetus.audience.InterfaceAudience; + +/** + * a balancer which is only used in maintenance mode. + */ +@InterfaceAudience.Private +public class MaintenanceLoadBalancer extends Configured implements LoadBalancer { + + private volatile boolean stopped = false; + + @Override + public void stop(String why) { + stopped = true; + } + + @Override + public boolean isStopped() { + return stopped; + } + + @Override + public void setClusterMetrics(ClusterMetrics st) { + } + + @Override + public void setMasterServices(MasterServices masterServices) { + } + + @Override + public List balanceCluster( + Map>> loadOfAllTable) throws IOException { + // do not need to balance in maintenance mode + return Collections.emptyList(); + } + + @Override + public List balanceTable(TableName tableName, + Map> loadOfOneTable) { + return Collections.emptyList(); + } + + private Map> assign(Collection regions, + List servers) { + // should only have 1 region server in maintenance mode + assert servers.size() == 1; + List systemRegions = + regions.stream().filter(r -> r.getTable().isSystemTable()).collect(Collectors.toList()); + if (!systemRegions.isEmpty()) { + return Collections.singletonMap(servers.get(0), systemRegions); + } else { + return Collections.emptyMap(); + } + } + + @Override + public Map> roundRobinAssignment(List regions, + List servers) throws IOException { + return assign(regions, servers); + } + + @Override + public Map> retainAssignment(Map regions, + List servers) throws IOException { + return assign(regions.keySet(), servers); + } + + @Override + public ServerName randomAssignment(RegionInfo regionInfo, List servers) + throws IOException { + // should only have 1 region server in maintenance mode + assert servers.size() == 1; + return regionInfo.getTable().isSystemTable() ? servers.get(0) : null; + } + + @Override + public void initialize() { + } + + @Override + public void regionOnline(RegionInfo regionInfo, ServerName sn) { + } + + @Override + public void regionOffline(RegionInfo regionInfo) { + } + + @Override + public void onConfigurationChange(Configuration conf) { + } + + @Override + public void postMasterStartupInitialize() { + } + + @Override + public void updateBalancerStatus(boolean status) { + } +} diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterRepairMode.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterRepairMode.java index 6e8544828f8..8301e0abe71 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterRepairMode.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterRepairMode.java @@ -18,9 +18,11 @@ package org.apache.hadoop.hbase.master; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; + import java.util.Arrays; +import java.util.concurrent.TimeUnit; import java.util.stream.Stream; import java.util.stream.StreamSupport; import org.apache.hadoop.conf.Configuration; @@ -29,6 +31,7 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.StartMiniClusterOption; import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.AsyncTable; import org.apache.hadoop.hbase.client.Connection; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.Result; @@ -48,12 +51,12 @@ import org.junit.rules.TestName; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Category({MasterTests.class, LargeTests.class}) +@Category({ MasterTests.class, LargeTests.class }) public class TestMasterRepairMode { @ClassRule public static final HBaseClassTestRule CLASS_RULE = - HBaseClassTestRule.forClass(TestMasterRepairMode.class); + HBaseClassTestRule.forClass(TestMasterRepairMode.class); @Rule public TestName name = new TestName(); @@ -84,16 +87,14 @@ public class TestMasterRepairMode { public void testNewCluster() throws Exception { enableMaintenanceMode(); - TEST_UTIL.startMiniCluster(StartMiniClusterOption.builder() - .numRegionServers(0) - .numDataNodes(3) - .build()); + TEST_UTIL.startMiniCluster( + StartMiniClusterOption.builder().numRegionServers(0).numDataNodes(3).build()); Connection conn = TEST_UTIL.getConnection(); assertTrue(conn.getAdmin().isMasterInMaintenanceMode()); try (Table table = conn.getTable(TableName.META_TABLE_NAME); - ResultScanner scanner = table.getScanner(new Scan())) { + ResultScanner scanner = table.getScanner(new Scan())) { assertNotNull("Could not read meta.", scanner.next()); } } @@ -113,25 +114,26 @@ public class TestMasterRepairMode { LOG.info("Starting master-only"); enableMaintenanceMode(); - TEST_UTIL.startMiniHBaseCluster(StartMiniClusterOption.builder() - .numRegionServers(0).createRootDir(false).build()); + TEST_UTIL.startMiniHBaseCluster( + StartMiniClusterOption.builder().numRegionServers(0).createRootDir(false).build()); Connection conn = TEST_UTIL.getConnection(); assertTrue(conn.getAdmin().isMasterInMaintenanceMode()); try (Table table = conn.getTable(TableName.META_TABLE_NAME); - ResultScanner scanner = table.getScanner(HConstants.TABLE_FAMILY); - Stream results = StreamSupport.stream(scanner.spliterator(), false)) { + ResultScanner scanner = table.getScanner(HConstants.TABLE_FAMILY); + Stream results = StreamSupport.stream(scanner.spliterator(), false)) { assertTrue("Did not find user table records while reading hbase:meta", - results.anyMatch(r -> Arrays.equals(r.getRow(), testRepairMode.getName()))); - } - - try (Table table = conn.getTable(testRepairMode); - ResultScanner scanner = table.getScanner(new Scan())) { - scanner.next(); - fail("Should not be able to access user-space tables in repair mode."); - } catch (Exception e) { - // Expected + results.anyMatch(r -> Arrays.equals(r.getRow(), testRepairMode.getName()))); } + // use async table so we can set the timeout and retry value to let the operation fail fast + AsyncTable table = conn.toAsyncConnection().getTableBuilder(testRepairMode) + .setScanTimeout(5, TimeUnit.SECONDS).setMaxRetries(2).build(); + assertThrows("Should not be able to access user-space tables in repair mode.", Exception.class, + () -> { + try (ResultScanner scanner = table.getScanner(new Scan())) { + scanner.next(); + } + }); } }