Apache
/
hbase
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HBASE-25093 the RSGroupBasedLoadBalancer#retainAssignment throws NPE (#2450) 

Signed-off-by: Duo Zhang <zhangduo@apache.org>
This commit is contained in:
niuyulin 2020-10-11 22:02:37 -05:00 committed by GitHub
parent 3d1aaa6632
commit 8eea052359
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 45 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
View File

@ -22,12 +22,14 @@ import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.PRIMARY;
import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.SECONDARY;
import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.TERTIARY;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HBaseIOException;
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
@ -161,6 +163,7 @@ public class FavoredNodeLoadBalancer extends BaseLoadBalancer implements Favored
}
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> roundRobinAssignment(List<RegionInfo> regions,
List<ServerName> servers) throws HBaseIOException {
Map<ServerName, List<RegionInfo>> assignmentMap;

5
hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
View File

@ -18,7 +18,7 @@
*/
package org.apache.hadoop.hbase.master;
import edu.umd.cs.findbugs.annotations.Nullable;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.List;
import java.util.Map;
@ -110,6 +110,7 @@ public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObse
* Perform a Round Robin assignment of regions.
* @return Map of servername to regioninfos
*/
@NonNull
Map<ServerName, List<RegionInfo>> roundRobinAssignment(List<RegionInfo> regions,
List<ServerName> servers) throws IOException;
@ -117,7 +118,7 @@ public interface LoadBalancer extends Configurable, Stoppable, ConfigurationObse
* Assign regions to the previously hosting region server
* @return List of plans
*/
@Nullable
@NonNull
Map<ServerName, List<RegionInfo>> retainAssignment(Map<RegionInfo, ServerName> regions,
List<ServerName> servers) throws IOException;

6
hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
View File

@ -2165,12 +2165,8 @@ public class AssignmentManager {
final ProcedureEvent<?>[] events = new ProcedureEvent[regions.size()];
final long st = System.currentTimeMillis();
if (plan == null) {
throw new HBaseIOException("unable to compute plans for regions=" + regions.size());
}
if (plan.isEmpty()) {
return;
throw new HBaseIOException("unable to compute plans for regions=" + regions.size());
}
int evcount = 0;

27
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/BaseLoadBalancer.java
View File

@ -18,6 +18,7 @@
*/
package org.apache.hadoop.hbase.master.balancer;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
@ -1132,11 +1133,9 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
* If master is configured to carry system tables only, in here is
* where we figure what to assign it.
*/
@NonNull
protected Map<ServerName, List<RegionInfo>> assignMasterSystemRegions(
Collection<RegionInfo> regions, List<ServerName> servers) {
if (servers == null || regions == null || regions.isEmpty()) {
return null;
}
Map<ServerName, List<RegionInfo>> assignments = new TreeMap<>();
if (this.maintenanceMode || this.onlySystemTablesOnMaster) {
if (masterServerName != null && servers.contains(masterServerName)) {
@ -1267,15 +1266,16 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
*
* @param regions all regions
* @param servers all servers
* @return map of server to the regions it should take, or null if no
* assignment is possible (ie. no regions or no servers)
* @return map of server to the regions it should take, or emptyMap if no
* assignment is possible (ie. no servers)
*/
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> roundRobinAssignment(List<RegionInfo> regions,
List<ServerName> servers) throws HBaseIOException {
metricsBalancer.incrMiscInvocations();
Map<ServerName, List<RegionInfo>> assignments = assignMasterSystemRegions(regions, servers);
if (assignments != null && !assignments.isEmpty()) {
if (!assignments.isEmpty()) {
servers = new ArrayList<>(servers);
// Guarantee not to put other regions on master
servers.remove(masterServerName);
@ -1285,14 +1285,17 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
regions.removeAll(masterRegions);
}
}
if (this.maintenanceMode || regions == null || regions.isEmpty()) {
/**
* only need assign system table
*/
if (this.maintenanceMode || regions.isEmpty()) {
return assignments;
}
int numServers = servers == null ? 0 : servers.size();
if (numServers == 0) {
LOG.warn("Wanted to do round robin assignment but no servers to assign to");
return null;
return Collections.emptyMap();
}
// TODO: instead of retainAssignment() and roundRobinAssignment(), we should just run the
@ -1407,15 +1410,17 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
*
* @param regions regions and existing assignment from meta
* @param servers available servers
* @return map of servers and regions to be assigned to them
* @return map of servers and regions to be assigned to them, or emptyMap if no
* assignment is possible (ie. no servers)
*/
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> retainAssignment(Map<RegionInfo, ServerName> regions,
List<ServerName> servers) throws HBaseIOException {
// Update metrics
metricsBalancer.incrMiscInvocations();
Map<ServerName, List<RegionInfo>> assignments = assignMasterSystemRegions(regions.keySet(), servers);
if (assignments != null && !assignments.isEmpty()) {
if (!assignments.isEmpty()) {
servers = new ArrayList<>(servers);
// Guarantee not to put other regions on master
servers.remove(masterServerName);
@ -1430,7 +1435,7 @@ public abstract class BaseLoadBalancer implements LoadBalancer {
int numServers = servers == null ? 0 : servers.size();
if (numServers == 0) {
LOG.warn("Wanted to do retain assignment but no servers to assign to");
return null;
return Collections.emptyMap();
}
if (numServers == 1) { // Only one server, nothing fancy we can do here
ServerName server = servers.get(0);

10
hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
View File

@ -23,6 +23,7 @@ import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.PRIMARY;
import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.SECONDARY;
import static org.apache.hadoop.hbase.favored.FavoredNodesPlan.Position.TERTIARY;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collection;
@ -31,6 +32,7 @@ import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import org.apache.hadoop.hbase.HBaseIOException;
import org.apache.hadoop.hbase.ServerMetrics;
import org.apache.hadoop.hbase.ServerName;
@ -109,6 +111,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements
* secondary and tertiary as per favored nodes constraints.
*/
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> roundRobinAssignment(List<RegionInfo> regions,
List<ServerName> servers) throws HBaseIOException {
@ -116,7 +119,7 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements
Set<RegionInfo> regionSet = Sets.newHashSet(regions);
Map<ServerName, List<RegionInfo>> assignmentMap = assignMasterSystemRegions(regions, servers);
if (assignmentMap != null && !assignmentMap.isEmpty()) {
if (!assignmentMap.isEmpty()) {
servers = new ArrayList<>(servers);
// Guarantee not to put other regions on master
servers.remove(masterServerName);
@ -367,14 +370,15 @@ public class FavoredStochasticBalancer extends StochasticLoadBalancer implements
* Reuse BaseLoadBalancer's retainAssignment, but generate favored nodes when its missing.
*/
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> retainAssignment(Map<RegionInfo, ServerName> regions,
List<ServerName> servers) throws HBaseIOException {
Map<ServerName, List<RegionInfo>> assignmentMap = Maps.newHashMap();
Map<ServerName, List<RegionInfo>> result = super.retainAssignment(regions, servers);
if (result == null || result.isEmpty()) {
if (result.isEmpty()) {
LOG.warn("Nothing to assign to, probably no servers or no regions");
return null;
return result;
}
// Guarantee not to put other regions on master

23
hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
View File

@ -17,6 +17,7 @@
*/
package org.apache.hadoop.hbase.rsgroup;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
@ -174,25 +175,25 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer {
}
@Override
public Map<ServerName, List<RegionInfo>> roundRobinAssignment(
List<RegionInfo> regions, List<ServerName> servers) throws IOException {
@NonNull
public Map<ServerName, List<RegionInfo>> roundRobinAssignment(List<RegionInfo> regions,
List<ServerName> servers) throws IOException {
Map<ServerName, List<RegionInfo>> assignments = Maps.newHashMap();
List<Pair<List<RegionInfo>, List<ServerName>>> pairs =
generateGroupAssignments(regions, servers);
for (Pair<List<RegionInfo>, List<ServerName>> pair : pairs) {
Map<ServerName, List<RegionInfo>> result = this.internalBalancer
.roundRobinAssignment(pair.getFirst(), pair.getSecond());
if (result != null) {
result.forEach((server, regionInfos) ->
assignments.computeIfAbsent(server, s -> Lists.newArrayList()).addAll(regionInfos));
}
Map<ServerName, List<RegionInfo>> result =
this.internalBalancer.roundRobinAssignment(pair.getFirst(), pair.getSecond());
result.forEach((server, regionInfos) -> assignments
.computeIfAbsent(server, s -> Lists.newArrayList()).addAll(regionInfos));
}
return assignments;
}
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> retainAssignment(Map<RegionInfo, ServerName> regions,
List<ServerName> servers) throws HBaseIOException {
List<ServerName> servers) throws HBaseIOException {
try {
Map<ServerName, List<RegionInfo>> assignments = new TreeMap<>();
List<Pair<List<RegionInfo>, List<ServerName>>> pairs =
@ -203,8 +204,8 @@ public class RSGroupBasedLoadBalancer implements LoadBalancer {
regionList.forEach(r -> currentAssignmentMap.put(r, regions.get(r)));
Map<ServerName, List<RegionInfo>> pairResult =
this.internalBalancer.retainAssignment(currentAssignmentMap, pair.getSecond());
pairResult.forEach((server, rs) ->
assignments.computeIfAbsent(server, s -> Lists.newArrayList()).addAll(rs));
pairResult.forEach((server, rs) -> assignments
.computeIfAbsent(server, s -> Lists.newArrayList()).addAll(rs));
}
return assignments;
} catch (IOException e) {

3
hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
View File

@ -21,8 +21,10 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import edu.umd.cs.findbugs.annotations.NonNull;
import java.util.List;
import java.util.Map;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder;
@ -280,6 +282,7 @@ public class TestZooKeeper {
static boolean retainAssignCalled = false;
@Override
@NonNull
public Map<ServerName, List<RegionInfo>> retainAssignment(
Map<RegionInfo, ServerName> regions, List<ServerName> servers) throws HBaseIOException {
retainAssignCalled = true;