wchevreuil commented on a change in pull request #2430:
URL: https://github.com/apache/hbase/pull/2430#discussion_r491893347



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
##########
@@ -63,7 +121,7 @@ protected synchronized void disconnect() {
    * A private method used to re-establish a zookeeper session with a peer 
cluster.
    * @param ke
    */
-  protected void reconnect(KeeperException ke) {
+  private void reconnect(KeeperException ke) {

Review comment:
       Can we keep all accessors to ZKWatcher `protected` or at least, `package 
private`? In some cases, custom HBaseReplicationEndpoint implementations may 
need to access those.

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
##########
@@ -150,13 +202,19 @@ public boolean isAborted() {
 
   /**
    * Get the list of all the region servers from the specified peer
-   * @param zkw zk connection to use
+   *
    * @return list of region server addresses or an empty list if the slave is 
unavailable
    */
-  protected static List<ServerName> fetchSlavesAddresses(ZKWatcher zkw)
-      throws KeeperException {
-    List<String> children = ZKUtil.listChildrenAndWatchForNewChildren(zkw,
-            zkw.getZNodePaths().rsZNode);
+  private List<ServerName> fetchSlavesAddresses() {

Review comment:
       Same as my previous comment. Would be nice to allow for custom 
implementations to access this method. 




----------------------------------------------------------------
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.

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


Reply via email to