You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2020/10/12 03:02:58 UTC
[hbase] branch master updated: HBASE-25093 the
RSGroupBasedLoadBalancer#retainAssignment throws NPE (#2450)
This is an automated email from the ASF dual-hosted git repository.
zhangduo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/master by this push:
new 8eea052 HBASE-25093 the RSGroupBasedLoadBalancer#retainAssignment throws NPE (#2450)
8eea052 is described below
commit 8eea05235929a16706a83b759d98f9336f10fd49
Author: niuyulin <ny...@163.com>
AuthorDate: Sun Oct 11 22:02:37 2020 -0500
HBASE-25093 the RSGroupBasedLoadBalancer#retainAssignment throws NPE (#2450)
Signed-off-by: Duo Zhang <zh...@apache.org>
---
.../hbase/favored/FavoredNodeLoadBalancer.java | 3 +++
.../apache/hadoop/hbase/master/LoadBalancer.java | 5 ++--
.../hbase/master/assignment/AssignmentManager.java | 6 +----
.../hbase/master/balancer/BaseLoadBalancer.java | 27 +++++++++++++---------
.../master/balancer/FavoredStochasticBalancer.java | 10 +++++---
.../hbase/rsgroup/RSGroupBasedLoadBalancer.java | 23 +++++++++---------
.../org/apache/hadoop/hbase/TestZooKeeper.java | 3 +++
7 files changed, 45 insertions(+), 32 deletions(-)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
index 8cde76e..60a2c6c 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeLoadBalancer.java
@@ -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;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
index b7ec1a3..90cb394 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/LoadBalancer.java
@@ -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;
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
index fb64514..f23d170 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignmentManager.java
@@ -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;
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 6a27a6a..a47bff2 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
@@ -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);
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
index 57fd73d..4108a5a 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/balancer/FavoredStochasticBalancer.java
@@ -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
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
index 50ddb41..db61c01 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/rsgroup/RSGroupBasedLoadBalancer.java
@@ -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) {
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
index 4bbb3ed..c0eacae 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestZooKeeper.java
@@ -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;