From 5af4b3d5c86d187be31576153c3bc0eaf46038be Mon Sep 17 00:00:00 2001 From: Andrew Purtell Date: Mon, 18 Jul 2022 15:35:43 -0700 Subject: [PATCH] HBASE-27202 Clean up error-prone findings in hbase-balancer (#4623) Signed-off-by: Duo Zhang --- .../hbase/favored/FavoredNodeAssignmentHelper.java | 12 +++++------- .../hadoop/hbase/favored/FavoredNodesPlan.java | 4 ++-- .../hbase/master/AssignmentVerificationReport.java | 6 +++--- .../org/apache/hadoop/hbase/master/RegionPlan.java | 5 +---- .../hbase/master/balancer/BalancerClusterState.java | 2 +- .../balancer/RegionHDFSBlockLocationFinder.java | 1 + 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeAssignmentHelper.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeAssignmentHelper.java index 006ff2e731d..68b08544196 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeAssignmentHelper.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodeAssignmentHelper.java @@ -111,7 +111,7 @@ public class FavoredNodeAssignmentHelper { break; } } - serverList.add((sn)); + serverList.add(sn); this.regionServerToRackMap.put(sn.getHostname(), rackName); } } @@ -235,7 +235,7 @@ public class FavoredNodeAssignmentHelper { if (numIterations % rackList.size() == 0) { if (++serverIndex >= maxRackSize) serverIndex = 0; } - if ((++rackIndex) >= rackList.size()) { + if (++rackIndex >= rackList.size()) { rackIndex = 0; // reset the rack index to 0 } } else break; @@ -259,7 +259,7 @@ public class FavoredNodeAssignmentHelper { if (numIterations % rackList.size() == 0) { ++serverIndex; } - if ((++rackIndex) >= rackList.size()) { + if (++rackIndex >= rackList.size()) { rackIndex = 0; // reset the rack index to 0 } } @@ -298,7 +298,7 @@ public class FavoredNodeAssignmentHelper { if (getTotalNumberOfRacks() == 1) { favoredNodes = singleRackCase(regionInfo, primaryRS, primaryRack); } else { - favoredNodes = multiRackCase(regionInfo, primaryRS, primaryRack); + favoredNodes = multiRackCase(primaryRS, primaryRack); } return favoredNodes; } @@ -483,14 +483,12 @@ public class FavoredNodeAssignmentHelper { * has only one region server, then we place primary and tertiary on one rack and secondary on * another. The aim is two distribute the three favored nodes on >= 2 racks. TODO: see how we can * use generateMissingFavoredNodeMultiRack API here - * @param regionInfo Region for which we are trying to generate FN * @param primaryRS The primary favored node. * @param primaryRack The rack of the primary favored node. * @return Array containing secondary and tertiary favored nodes. * @throws IOException Signals that an I/O exception has occurred. */ - private ServerName[] multiRackCase(RegionInfo regionInfo, ServerName primaryRS, - String primaryRack) throws IOException { + private ServerName[] multiRackCase(ServerName primaryRS, String primaryRack) throws IOException { List favoredNodes = Lists.newArrayList(primaryRS); // Create the secondary and tertiary pair diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesPlan.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesPlan.java index 224a32c222a..4c6f2b3cc27 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesPlan.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/favored/FavoredNodesPlan.java @@ -99,7 +99,7 @@ public class FavoredNodesPlan { } /** - * @return the mapping between each region to its favored region server list + * Return the mapping between each region to its favored region server list. */ public Map> getAssignmentMap() { // Make a deep copy so changes don't harm our copy of favoredNodesMap. @@ -119,7 +119,7 @@ public class FavoredNodesPlan { if (o == null) { return false; } - if (getClass() != o.getClass()) { + if (!(o instanceof FavoredNodesPlan)) { return false; } // To compare the map from object o is identical to current assignment map. diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/AssignmentVerificationReport.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/AssignmentVerificationReport.java index 2c49e26e9cf..8858e13da70 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/AssignmentVerificationReport.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/AssignmentVerificationReport.java @@ -409,8 +409,8 @@ public class AssignmentVerificationReport { } /** - * @return list which contains just 3 elements: average dispersion score, max dispersion score and - * min dispersion score as first, second and third element respectively. + * Return a list which contains 3 elements: average dispersion score, max dispersion score and min + * dispersion score as first, second and third elements, respectively. */ public List getDispersionInformation() { List dispersion = new ArrayList<>(); @@ -578,7 +578,7 @@ public class AssignmentVerificationReport { } int i = 0; for (ServerName addr : serverSet) { - if ((i++) % 3 == 0) { + if (i++ % 3 == 0) { System.out.print("\n\t\t\t"); } System.out.print(addr.getAddress() + " ; "); diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java index d6909dc2802..5632fcc02ff 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/RegionPlan.java @@ -148,10 +148,7 @@ public class RegionPlan implements Comparable { if (this == obj) { return true; } - if (obj == null) { - return false; - } - if (getClass() != obj.getClass()) { + if (!(obj instanceof RegionPlan)) { return false; } RegionPlan other = (RegionPlan) obj; diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java index a54a410fcdf..a7ae8b4d1a5 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/BalancerClusterState.java @@ -218,7 +218,7 @@ class BalancerClusterState { colocatedReplicaCountsPerHost = new Int2IntCounterMap[numHosts]; colocatedReplicaCountsPerRack = new Int2IntCounterMap[numRacks]; - int tableIndex = 0, regionIndex = 0, regionPerServerIndex = 0; + int regionIndex = 0, regionPerServerIndex = 0; for (Map.Entry> entry : clusterState.entrySet()) { if (entry.getKey() == null) { diff --git a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/RegionHDFSBlockLocationFinder.java b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/RegionHDFSBlockLocationFinder.java index 1d0f21a5088..0a14e5b0b51 100644 --- a/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/RegionHDFSBlockLocationFinder.java +++ b/hbase-balancer/src/main/java/org/apache/hadoop/hbase/master/balancer/RegionHDFSBlockLocationFinder.java @@ -246,6 +246,7 @@ class RegionHDFSBlockLocationFinder extends Configured { */ @RestrictedApi(explanation = "Should only be called in tests", link = "", allowedOnPath = ".*/src/test/.*|.*/RegionHDFSBlockLocationFinder.java") + @SuppressWarnings("MixedMutabilityReturnType") List mapHostNameToServerName(List hosts) { if (hosts == null || status == null) { if (hosts == null) {