From d6aff6cbae5157b99c3e1c83472c7d3243a131db Mon Sep 17 00:00:00 2001 From: XinSun Date: Sun, 7 Feb 2021 17:13:47 +0800 Subject: [PATCH] HBASE-25553 It is better for ReplicationTracker.getListOfRegionServers to return ServerName instead of String (#2928) Signed-off-by: Wellington Chevreuil Signed-off-by: Viraj Jasani --- .../hbase/replication/ReplicationTracker.java | 7 ++++--- .../replication/ReplicationTrackerZKImpl.java | 16 ++++++++++------ .../regionserver/DumpReplicationQueues.java | 4 ++-- .../regionserver/ReplicationSourceManager.java | 3 +-- .../TestReplicationTrackerZKImpl.java | 18 +++++++++--------- 5 files changed, 26 insertions(+), 22 deletions(-) diff --git a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTracker.java b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTracker.java index 93a32630d55..a33e23dc96b 100644 --- a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTracker.java +++ b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTracker.java @@ -20,6 +20,7 @@ package org.apache.hadoop.hbase.replication; import java.util.List; +import org.apache.hadoop.hbase.ServerName; import org.apache.yetus.audience.InterfaceAudience; /** @@ -37,13 +38,13 @@ public interface ReplicationTracker { * Register a replication listener to receive replication events. * @param listener the listener to register */ - public void registerListener(ReplicationListener listener); + void registerListener(ReplicationListener listener); - public void removeListener(ReplicationListener listener); + void removeListener(ReplicationListener listener); /** * Returns a list of other live region servers in the cluster. * @return List of region servers. */ - public List getListOfRegionServers(); + List getListOfRegionServers(); } diff --git a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTrackerZKImpl.java b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTrackerZKImpl.java index 54c9c2cdc0a..6fc3c452723 100644 --- a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTrackerZKImpl.java +++ b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ReplicationTrackerZKImpl.java @@ -20,7 +20,10 @@ package org.apache.hadoop.hbase.replication; import java.util.ArrayList; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; +import java.util.stream.Collectors; + import org.apache.hadoop.hbase.Abortable; +import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.Stoppable; import org.apache.hadoop.hbase.zookeeper.ZKListener; import org.apache.hadoop.hbase.zookeeper.ZKUtil; @@ -49,7 +52,7 @@ public class ReplicationTrackerZKImpl implements ReplicationTracker { // listeners to be notified private final List listeners = new CopyOnWriteArrayList<>(); // List of all the other region servers in this cluster - private final ArrayList otherRegionServers = new ArrayList<>(); + private final List otherRegionServers = new ArrayList<>(); public ReplicationTrackerZKImpl(ZKWatcher zookeeper, Abortable abortable, Stoppable stopper) { this.zookeeper = zookeeper; @@ -74,10 +77,10 @@ public class ReplicationTrackerZKImpl implements ReplicationTracker { * Return a snapshot of the current region servers. */ @Override - public List getListOfRegionServers() { + public List getListOfRegionServers() { refreshOtherRegionServersList(false); - List list = null; + List list = null; synchronized (otherRegionServers) { list = new ArrayList<>(otherRegionServers); } @@ -162,7 +165,7 @@ public class ReplicationTrackerZKImpl implements ReplicationTracker { * if it was empty), false if the data was missing in ZK */ private boolean refreshOtherRegionServersList(boolean watch) { - List newRsList = getRegisteredRegionServers(watch); + List newRsList = getRegisteredRegionServers(watch); if (newRsList == null) { return false; } else { @@ -178,7 +181,7 @@ public class ReplicationTrackerZKImpl implements ReplicationTracker { * Get a list of all the other region servers in this cluster and set a watch * @return a list of server nanes */ - private List getRegisteredRegionServers(boolean watch) { + private List getRegisteredRegionServers(boolean watch) { List result = null; try { if (watch) { @@ -190,6 +193,7 @@ public class ReplicationTrackerZKImpl implements ReplicationTracker { } catch (KeeperException e) { this.abortable.abort("Get list of registered region servers", e); } - return result; + return result == null ? null : + result.stream().map(ServerName::parseServerName).collect(Collectors.toList()); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/DumpReplicationQueues.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/DumpReplicationQueues.java index cc0d9bbaa2e..92c57a89d6b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/DumpReplicationQueues.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/DumpReplicationQueues.java @@ -308,7 +308,7 @@ public class DumpReplicationQueues extends Configured implements Tool { queueStorage = ReplicationStorageFactory.getReplicationQueueStorage(zkw, getConf()); replicationTracker = ReplicationFactory.getReplicationTracker(zkw, new WarnOnlyAbortable(), new WarnOnlyStoppable()); - Set liveRegionServers = new HashSet<>(replicationTracker.getListOfRegionServers()); + Set liveRegionServers = new HashSet<>(replicationTracker.getListOfRegionServers()); // Loops each peer on each RS and dumps the queues List regionservers = queueStorage.getListOfReplicators(); @@ -317,7 +317,7 @@ public class DumpReplicationQueues extends Configured implements Tool { } for (ServerName regionserver : regionservers) { List queueIds = queueStorage.getAllQueues(regionserver); - if (!liveRegionServers.contains(regionserver.getServerName())) { + if (!liveRegionServers.contains(regionserver)) { deadRegionServers.add(regionserver.getServerName()); } for (String queueId : queueIds) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java index c1166802b0e..303a091ac98 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceManager.java @@ -283,8 +283,7 @@ public class ReplicationSourceManager implements ReplicationListener { if (currentReplicators == null || currentReplicators.isEmpty()) { return; } - List otherRegionServers = replicationTracker.getListOfRegionServers().stream() - .map(ServerName::valueOf).collect(Collectors.toList()); + List otherRegionServers = replicationTracker.getListOfRegionServers(); LOG.info( "Current list of replicators: " + currentReplicators + " other RSs: " + otherRegionServers); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationTrackerZKImpl.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationTrackerZKImpl.java index 1500a717c40..da82e19f2ab 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationTrackerZKImpl.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationTrackerZKImpl.java @@ -115,26 +115,26 @@ public class TestReplicationTrackerZKImpl { assertEquals(0, rt.getListOfRegionServers().size()); // 1 region server - ZKUtil.createWithParents(zkw, - ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, "hostname1.example.org:1234")); - List rss = rt.getListOfRegionServers(); + ZKUtil.createWithParents(zkw, ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, + "hostname1.example.org,1234,1611218678009")); + List rss = rt.getListOfRegionServers(); assertEquals(rss.toString(), 1, rss.size()); // 2 region servers - ZKUtil.createWithParents(zkw, - ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, "hostname2.example.org:1234")); + ZKUtil.createWithParents(zkw, ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, + "hostname2.example.org,1234,1611218678009")); rss = rt.getListOfRegionServers(); assertEquals(rss.toString(), 2, rss.size()); // 1 region server - ZKUtil.deleteNode(zkw, - ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, "hostname2.example.org:1234")); + ZKUtil.deleteNode(zkw, ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, + "hostname2.example.org,1234,1611218678009")); rss = rt.getListOfRegionServers(); assertEquals(1, rss.size()); // 0 region server - ZKUtil.deleteNode(zkw, - ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, "hostname1.example.org:1234")); + ZKUtil.deleteNode(zkw, ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, + "hostname1.example.org,1234,1611218678009")); rss = rt.getListOfRegionServers(); assertEquals(rss.toString(), 0, rss.size()); }