anmolnar commented on code in PR #5825:
URL: https://github.com/apache/hbase/pull/5825#discussion_r1575308563


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java:
##########
@@ -224,36 +184,27 @@ public boolean isAborted() {
    * Get the list of all the region servers from the specified peer
    * @return list of region server addresses or an empty list if the slave is 
unavailable
    */
-  protected List<ServerName> fetchSlavesAddresses() {
-    List<String> children = null;
+  // will be overrided in tests so protected
+  protected Collection<ServerName> fetchPeerAddresses() {
     try {
-      synchronized (zkwLock) {
-        children = ZKUtil.listChildrenAndWatchForNewChildren(zkw, 
zkw.getZNodePaths().rsZNode);
-      }
-    } catch (KeeperException ke) {
-      if (LOG.isDebugEnabled()) {
-        LOG.debug("Fetch slaves addresses failed", ke);
-      }
-      reconnect(ke);
-    }
-    if (children == null) {
+      return FutureUtils.get(conn.getAdmin().getRegionServers(true));
+    } catch (IOException e) {
+      LOG.debug("Fetch peer addresses failed", e);
       return Collections.emptyList();
     }
-    List<ServerName> addresses = new ArrayList<>(children.size());
-    for (String child : children) {
-      addresses.add(ServerName.parseServerName(child));
-    }
-    return addresses;
   }
 
   protected synchronized void chooseSinks() {

Review Comment:
   You have removed `PeerRegionServerListener` and I don't see anything 
replacing it. How will the replication endpoint notice if region servers are 
changed?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to