You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by su...@apache.org on 2021/02/07 09:14:15 UTC

[hbase] branch master updated: HBASE-25553 It is better for ReplicationTracker.getListOfRegionServers to return ServerName instead of String (#2928)

This is an automated email from the ASF dual-hosted git repository.

sunxin 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 d6aff6c  HBASE-25553 It is better for ReplicationTracker.getListOfRegionServers to return ServerName instead of String (#2928)
d6aff6c is described below

commit d6aff6cbae5157b99c3e1c83472c7d3243a131db
Author: XinSun <dd...@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 <wc...@apache.org>
    Signed-off-by: Viraj Jasani <vj...@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 93a3263..a33e23d 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<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 cc0d9bb..92c57a8 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<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();
@@ -317,7 +317,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 c116680..303a091 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<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 1500a71..da82e19 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());
   }