Skip to content

Commit dc5539c

Browse files
rmdmattinglyRay Mattingly
authored andcommitted
HBASE-28146: Make ServerManager rsAdmins map thread safe (#5461)
Co-authored-by: Ray Mattingly <rmattingly@hubspot.com> Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Bryan Beaudreault <bbeaudreault@apache.org> (cherry picked from commit 1641a4a)
1 parent cd3a83f commit dc5539c

File tree

1 file changed

+7
-20
lines changed

1 file changed

+7
-20
lines changed

hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import java.net.InetAddress;
2424
import java.util.ArrayList;
2525
import java.util.Collections;
26-
import java.util.HashMap;
2726
import java.util.List;
2827
import java.util.Map;
2928
import java.util.Map.Entry;
@@ -44,6 +43,7 @@
4443
import org.apache.hadoop.hbase.YouAreDeadException;
4544
import org.apache.hadoop.hbase.client.ClusterConnection;
4645
import org.apache.hadoop.hbase.client.RegionInfo;
46+
import org.apache.hadoop.hbase.client.RetriesExhaustedException;
4747
import org.apache.hadoop.hbase.ipc.HBaseRpcController;
4848
import org.apache.hadoop.hbase.ipc.RemoteWithExtrasException;
4949
import org.apache.hadoop.hbase.ipc.RpcControllerFactory;
@@ -123,12 +123,6 @@ public class ServerManager {
123123
private final ConcurrentNavigableMap<ServerName, ServerMetrics> onlineServers =
124124
new ConcurrentSkipListMap<>();
125125

126-
/**
127-
* Map of admin interfaces per registered regionserver; these interfaces we use to control
128-
* regionservers out on the cluster
129-
*/
130-
private final Map<ServerName, AdminService.BlockingInterface> rsAdmins = new HashMap<>();
131-
132126
/** List of region servers that should not get any more new regions. */
133127
private final ArrayList<ServerName> drainingServers = new ArrayList<>();
134128

@@ -393,7 +387,6 @@ private ServerName findServerWithSameHostnamePortWithLock(final ServerName serve
393387
void recordNewServerWithLock(final ServerName serverName, final ServerMetrics sl) {
394388
LOG.info("Registering regionserver=" + serverName);
395389
this.onlineServers.put(serverName, sl);
396-
this.rsAdmins.remove(serverName);
397390
}
398391

399392
public RegionStoreSequenceIds getLastFlushedSequenceId(byte[] encodedRegionName) {
@@ -595,7 +588,6 @@ public synchronized void moveFromOnlineToDeadServers(final ServerName sn) {
595588
LOG.trace("Expiration of {} but server not online", sn);
596589
}
597590
}
598-
this.rsAdmins.remove(sn);
599591
}
600592

601593
/*
@@ -707,18 +699,13 @@ public static void closeRegionSilentlyAndWait(ClusterConnection connection, Serv
707699
* @throws RetriesExhaustedException wrapping a ConnectException if failed
708700
*/
709701
public AdminService.BlockingInterface getRsAdmin(final ServerName sn) throws IOException {
710-
AdminService.BlockingInterface admin = this.rsAdmins.get(sn);
711-
if (admin == null) {
712-
LOG.debug("New admin connection to " + sn.toString());
713-
if (sn.equals(master.getServerName()) && master instanceof HRegionServer) {
714-
// A master is also a region server now, see HBASE-10569 for details
715-
admin = ((HRegionServer) master).getRSRpcServices();
716-
} else {
717-
admin = this.connection.getAdmin(sn);
718-
}
719-
this.rsAdmins.put(sn, admin);
702+
LOG.debug("New admin connection to {}", sn);
703+
if (sn.equals(master.getServerName()) && master instanceof HRegionServer) {
704+
// A master is also a region server now, see HBASE-10569 for details
705+
return ((HRegionServer) master).getRSRpcServices();
706+
} else {
707+
return this.connection.getAdmin(sn);
720708
}
721-
return admin;
722709
}
723710

724711
/**

0 commit comments

Comments
 (0)