diff --git a/hbase-rsgroup/pom.xml b/hbase-rsgroup/pom.xml index b494a9afde7c..0bef6604783d 100644 --- a/hbase-rsgroup/pom.xml +++ b/hbase-rsgroup/pom.xml @@ -98,6 +98,12 @@ org.apache.hbase hbase-procedure + + org.apache.hbase + hbase-procedure + test-jar + test + org.apache.hbase hbase-protocol diff --git a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java index 9709fb550d9b..6767ac930940 100644 --- a/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java +++ b/hbase-rsgroup/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java @@ -170,23 +170,21 @@ public List balanceCluster(Map> cluster } @Override - public Map> roundRobinAssignment( - List regions, List servers) throws HBaseIOException { + public Map> roundRobinAssignment(List regions, + List servers) throws HBaseIOException { Map> assignments = Maps.newHashMap(); - ListMultimap regionMap = ArrayListMultimap.create(); - ListMultimap serverMap = ArrayListMultimap.create(); + ListMultimap regionMap = ArrayListMultimap.create(); + ListMultimap serverMap = ArrayListMultimap.create(); generateGroupMaps(regions, servers, regionMap, serverMap); - for(String groupKey : regionMap.keySet()) { + for (String groupKey : regionMap.keySet()) { if (regionMap.get(groupKey).size() > 0) { - Map> result = - this.internalBalancer.roundRobinAssignment( - regionMap.get(groupKey), - serverMap.get(groupKey)); - if(result != null) { - if(result.containsKey(LoadBalancer.BOGUS_SERVER_NAME) && - assignments.containsKey(LoadBalancer.BOGUS_SERVER_NAME)){ - assignments.get(LoadBalancer.BOGUS_SERVER_NAME).addAll( - result.get(LoadBalancer.BOGUS_SERVER_NAME)); + Map> result = this.internalBalancer + .roundRobinAssignment(regionMap.get(groupKey), serverMap.get(groupKey)); + if (result != null) { + if (result.containsKey(LoadBalancer.BOGUS_SERVER_NAME) && + assignments.containsKey(LoadBalancer.BOGUS_SERVER_NAME)) { + assignments.get(LoadBalancer.BOGUS_SERVER_NAME) + .addAll(result.get(LoadBalancer.BOGUS_SERVER_NAME)); } else { assignments.putAll(result); } @@ -197,24 +195,19 @@ public Map> roundRobinAssignment( } @Override - public Map> retainAssignment( - Map regions, List servers) throws HBaseIOException { + public Map> retainAssignment(Map regions, + List servers) throws HBaseIOException { try { Map> assignments = new TreeMap<>(); ListMultimap groupToRegion = ArrayListMultimap.create(); - Set misplacedRegions = getMisplacedRegions(regions); for (RegionInfo region : regions.keySet()) { - if (!misplacedRegions.contains(region)) { - String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable()); - if (groupName == null) { - LOG.debug("Group not found for table " + region.getTable() + ", using default"); - groupName = RSGroupInfo.DEFAULT_GROUP; - } - groupToRegion.put(groupName, region); + String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable()); + if (groupName == null) { + LOG.debug("Group not found for table " + region.getTable() + ", using default"); + groupName = RSGroupInfo.DEFAULT_GROUP; } + groupToRegion.put(groupName, region); } - // Now the "groupToRegion" map has only the regions which have correct - // assignments. for (String key : groupToRegion.keySet()) { Map currentAssignmentMap = new TreeMap(); List regionList = groupToRegion.get(key); @@ -223,34 +216,16 @@ public Map> retainAssignment( for (RegionInfo region : regionList) { currentAssignmentMap.put(region, regions.get(region)); } - if(candidateList.size() > 0) { - assignments.putAll(this.internalBalancer.retainAssignment( - currentAssignmentMap, candidateList)); + if (candidateList.size() > 0) { + assignments + .putAll(this.internalBalancer.retainAssignment(currentAssignmentMap, candidateList)); } else { if (LOG.isDebugEnabled()) { LOG.debug("No available servers to assign regions: {}", - RegionInfo.getShortNameToLog(regionList)); + RegionInfo.getShortNameToLog(regionList)); } assignments.computeIfAbsent(LoadBalancer.BOGUS_SERVER_NAME, s -> new ArrayList<>()) - .addAll(regionList); - } - } - - for (RegionInfo region : misplacedRegions) { - String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable()); - if (groupName == null) { - LOG.debug("Group not found for table " + region.getTable() + ", using default"); - groupName = RSGroupInfo.DEFAULT_GROUP; - } - RSGroupInfo info = rsGroupInfoManager.getRSGroup(groupName); - List candidateList = filterOfflineServers(info, servers); - ServerName server = this.internalBalancer.randomAssignment(region, - candidateList); - if (server != null) { - assignments.computeIfAbsent(server, s -> new ArrayList<>()).add(region); - } else { - assignments.computeIfAbsent(LoadBalancer.BOGUS_SERVER_NAME, s -> new ArrayList<>()) - .add(region); + .addAll(regionList); } } return assignments; @@ -269,11 +244,9 @@ public ServerName randomAssignment(RegionInfo region, return this.internalBalancer.randomAssignment(region, filteredServers); } - private void generateGroupMaps( - List regions, - List servers, - ListMultimap regionMap, - ListMultimap serverMap) throws HBaseIOException { + private void generateGroupMaps(List regions, List servers, + ListMultimap regionMap, ListMultimap serverMap) + throws HBaseIOException { try { for (RegionInfo region : regions) { String groupName = rsGroupInfoManager.getRSGroupOfTable(region.getTable()); @@ -307,63 +280,26 @@ private List filterOfflineServers(RSGroupInfo RSGroupInfo, /** * Filter servers based on the online servers. - * - * @param servers - * the servers - * @param onlineServers - * List of servers which are online. + *

+ * servers is actually a TreeSet (see {@link org.apache.hadoop.hbase.rsgroup.RSGroupInfo}), having + * its contains()'s time complexity as O(logn), which is good enough. + *

+ * TODO: consider using HashSet to pursue O(1) for contains() throughout the calling chain if + * needed. + * @param servers the servers + * @param onlineServers List of servers which are online. * @return the list */ - private List filterServers(Set

servers, - List onlineServers) { - /** - * servers is actually a TreeSet (see {@link org.apache.hadoop.hbase.rsgroup.RSGroupInfo}), - * having its contains()'s time complexity as O(logn), which is good enough. - * TODO: consider using HashSet to pursue O(1) for contains() throughout the calling chain - * if needed. */ + private List filterServers(Set
servers, List onlineServers) { ArrayList finalList = new ArrayList<>(); for (ServerName onlineServer : onlineServers) { if (servers.contains(onlineServer.getAddress())) { finalList.add(onlineServer); } } - return finalList; } - @VisibleForTesting - public Set getMisplacedRegions( - Map regions) throws IOException { - Set misplacedRegions = new HashSet<>(); - for(Map.Entry region : regions.entrySet()) { - RegionInfo regionInfo = region.getKey(); - ServerName assignedServer = region.getValue(); - String groupName = rsGroupInfoManager.getRSGroupOfTable(regionInfo.getTable()); - if (groupName == null) { - LOG.debug("Group not found for table " + regionInfo.getTable() + ", using default"); - groupName = RSGroupInfo.DEFAULT_GROUP; - } - RSGroupInfo info = rsGroupInfoManager.getRSGroup(groupName); - if (assignedServer == null) { - LOG.debug("There is no assigned server for {}", region); - continue; - } - RSGroupInfo otherInfo = rsGroupInfoManager.getRSGroupOfServer(assignedServer.getAddress()); - if (info == null && otherInfo == null) { - LOG.warn("Couldn't obtain rs group information for {} on {}", region, assignedServer); - continue; - } - if ((info == null || !info.containsServer(assignedServer.getAddress()))) { - LOG.debug("Found misplaced region: " + regionInfo.getRegionNameAsString() + - " on server: " + assignedServer + - " found in group: " + otherInfo + - " outside of group: " + (info == null ? "UNKNOWN" : info.getName())); - misplacedRegions.add(regionInfo); - } - } - return misplacedRegions; - } - private Pair>, List> correctAssignments( Map> existingAssignments) throws HBaseIOException{ // To return diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java index b60ca7ea2995..4b8820136a90 100644 --- a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/balancer/TestRSGroupBasedLoadBalancer.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hbase.master.balancer; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import java.util.ArrayList; @@ -36,7 +35,6 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.client.RegionInfo; -import org.apache.hadoop.hbase.client.RegionInfoBuilder; import org.apache.hadoop.hbase.master.LoadBalancer; import org.apache.hadoop.hbase.master.RegionPlan; import org.apache.hadoop.hbase.net.Address; @@ -131,21 +129,6 @@ public void testBulkAssignment() throws Exception { assertClusterAsBalanced(loadMap); } - @Test - public void testGetMisplacedRegions() throws Exception { - // Test case where region is not considered misplaced if RSGroupInfo cannot be determined - Map inputForTest = new HashMap<>(); - RegionInfo ri = RegionInfoBuilder.newBuilder(table0) - .setStartKey(new byte[16]) - .setEndKey(new byte[16]) - .setSplit(false) - .setRegionId(regionId++) - .build(); - inputForTest.put(ri, servers.iterator().next()); - Set misplacedRegions = loadBalancer.getMisplacedRegions(inputForTest); - assertFalse(misplacedRegions.contains(ri)); - } - /** * Test the cluster startup bulk assignment which attempts to retain assignment info. */ diff --git a/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPWithReplicasWithRSGroup.java b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPWithReplicasWithRSGroup.java new file mode 100644 index 000000000000..0acc603b503b --- /dev/null +++ b/hbase-rsgroup/src/test/java/org/apache/hadoop/hbase/master/procedure/TestSCPWithReplicasWithRSGroup.java @@ -0,0 +1,60 @@ +/** + * 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.procedure; + +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.HBaseClassTestRule; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.master.LoadBalancer; +import org.apache.hadoop.hbase.rsgroup.RSGroupAdminEndpoint; +import org.apache.hadoop.hbase.rsgroup.RSGroupBasedLoadBalancer; +import org.apache.hadoop.hbase.testclassification.MediumTests; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ MediumTests.class }) +public class TestSCPWithReplicasWithRSGroup extends TestSCPBase { + + @ClassRule + public static final HBaseClassTestRule CLASS_RULE = + HBaseClassTestRule.forClass(TestSCPWithReplicasWithRSGroup.class); + + @Override + protected void setupConf(Configuration conf) { + conf.setClass(HConstants.HBASE_MASTER_LOADBALANCER_CLASS, RSGroupBasedLoadBalancer.class, + LoadBalancer.class); + conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, RSGroupAdminEndpoint.class.getName()); + } + + @Override + protected void startMiniCluster() throws Exception { + this.util.startMiniCluster(4); + } + + @Override + protected int getRegionReplication() { + return 3; + } + + @Test + public void testCrashTargetRs() throws Exception { + testRecoveryAndDoubleExecution(false, false); + } +}