This is an automated email from the ASF dual-hosted git repository. sunxin pushed a commit to branch branch-2 in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-2 by this push: new 471f523 HBASE-25553 It is better for ReplicationTracker.getListOfRegionServers to return ServerName instead of String (#2928) 471f523 is described below commit 471f52350af8c14c55e2d5bcbeab8b36e9ab18e5 Author: XinSun <ddu...@gmail.com> AuthorDate: Sun Feb 7 17:13:47 2021 +0800 HBASE-25553 It is better for ReplicationTracker.getListOfRegionServers to return ServerName instead of String (#2928) Signed-off-by: Wellington Chevreuil <wchevre...@apache.org> Signed-off-by: Viraj Jasani <vjas...@apache.org> --- .../hadoop/hbase/replication/ReplicationTracker.java | 7 ++++--- .../hbase/replication/ReplicationTrackerZKImpl.java | 16 ++++++++++------ .../regionserver/DumpReplicationQueues.java | 4 ++-- .../regionserver/ReplicationSourceManager.java | 3 +-- .../replication/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 95bb5db..9370226 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 */ - 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<String> getListOfRegionServers(); + List<ServerName> 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 54c9c2c..6fc3c45 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<ReplicationListener> listeners = new CopyOnWriteArrayList<>(); // List of all the other region servers in this cluster - private final ArrayList<String> otherRegionServers = new ArrayList<>(); + private final List<ServerName> 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<String> getListOfRegionServers() { + public List<ServerName> getListOfRegionServers() { refreshOtherRegionServersList(false); - List<String> list = null; + List<ServerName> 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<String> newRsList = getRegisteredRegionServers(watch); + List<ServerName> 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<String> getRegisteredRegionServers(boolean watch) { + private List<ServerName> getRegisteredRegionServers(boolean watch) { List<String> 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 432dbcd..5201d6e 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 @@ -311,7 +311,7 @@ public class DumpReplicationQueues extends Configured implements Tool { queueStorage = ReplicationStorageFactory.getReplicationQueueStorage(zkw, getConf()); replicationTracker = ReplicationFactory.getReplicationTracker(zkw, new WarnOnlyAbortable(), new WarnOnlyStoppable()); - Set<String> liveRegionServers = new HashSet<>(replicationTracker.getListOfRegionServers()); + Set<ServerName> liveRegionServers = new HashSet<>(replicationTracker.getListOfRegionServers()); // Loops each peer on each RS and dumps the queues List<ServerName> regionservers = queueStorage.getListOfReplicators(); @@ -320,7 +320,7 @@ public class DumpReplicationQueues extends Configured implements Tool { } for (ServerName regionserver : regionservers) { List<String> 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 df3bd4b..5bd7157 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 @@ -266,8 +266,7 @@ public class ReplicationSourceManager implements ReplicationListener { if (currentReplicators == null || currentReplicators.isEmpty()) { return; } - List<ServerName> otherRegionServers = replicationTracker.getListOfRegionServers().stream() - .map(ServerName::valueOf).collect(Collectors.toList()); + List<ServerName> 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 9a94219..86743fe 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<String> rss = rt.getListOfRegionServers(); + ZKUtil.createWithParents(zkw, ZNodePaths.joinZNode(zkw.getZNodePaths().rsZNode, + "hostname1.example.org,1234,1611218678009")); + List<ServerName> 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()); }