You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ap...@apache.org on 2018/05/23 23:47:12 UTC

[1/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Repository: hbase
Updated Branches:
  refs/heads/branch-1 498f3bf95 -> 1b70763b9
  refs/heads/branch-1.2 66941d70b -> 8040c0ca7
  refs/heads/branch-1.3 94001b35a -> b50e14980
  refs/heads/branch-1.4 cd6397be6 -> 7182df3bd
  refs/heads/branch-2 12d75724d -> 60dcef289
  refs/heads/branch-2.0 86a9b80ff -> 6ecb44420
  refs/heads/master 3a805074a -> 9fbce1668


HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/1b70763b
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/1b70763b
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/1b70763b

Branch: refs/heads/branch-1
Commit: 1b70763b9ee6b23a0a3c9db0474c4c6654ce7189
Parents: 498f3bf
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:45:55 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/1b70763b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index 6485e4a..39a3f31 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.apache.zookeeper.KeeperException.SessionExpiredException;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Log LOG = LogFactory.getLog(HBaseReplicationEndpoint.class);
 
-  private ZooKeeperWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZooKeeperWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<ServerName>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -102,7 +103,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -114,7 +117,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZooKeeperWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -122,10 +127,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -163,13 +172,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[2/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/7182df3b
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/7182df3b
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/7182df3b

Branch: refs/heads/branch-1.4
Commit: 7182df3bd319472ac4223fb9cf48eb3bc1bba0a9
Parents: cd6397b
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:45:57 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/7182df3b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index 6485e4a..39a3f31 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.apache.zookeeper.KeeperException.SessionExpiredException;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Log LOG = LogFactory.getLog(HBaseReplicationEndpoint.class);
 
-  private ZooKeeperWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZooKeeperWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<ServerName>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -102,7 +103,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -114,7 +117,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZooKeeperWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -122,10 +127,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -163,13 +172,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[4/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8040c0ca
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8040c0ca
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8040c0ca

Branch: refs/heads/branch-1.2
Commit: 8040c0ca7696dfa776ee8449750279aa91c3fbd4
Parents: 66941d7
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:46:04 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/8040c0ca/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index 6485e4a..39a3f31 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.apache.zookeeper.KeeperException.SessionExpiredException;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Log LOG = LogFactory.getLog(HBaseReplicationEndpoint.class);
 
-  private ZooKeeperWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZooKeeperWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<ServerName>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -102,7 +103,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -114,7 +117,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZooKeeperWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -122,10 +127,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -163,13 +172,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[6/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/6ecb4442
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/6ecb4442
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/6ecb4442

Branch: refs/heads/branch-2.0
Commit: 6ecb4442088bb76bc37a1794376ce0bb08859cb0
Parents: 86a9b80
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:46:21 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/6ecb4442/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index bd5c529..8286f7d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.slf4j.LoggerFactory;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Logger LOG = LoggerFactory.getLogger(HBaseReplicationEndpoint.class);
 
-  private ZKWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZKWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -112,7 +113,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -124,7 +127,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZKWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -132,10 +137,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZKWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZKWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -173,13 +182,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[5/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/60dcef28
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/60dcef28
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/60dcef28

Branch: refs/heads/branch-2
Commit: 60dcef289b12e1650d715691eccdfa18481b432f
Parents: 12d7572
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:46:20 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/60dcef28/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index bd5c529..8286f7d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.slf4j.LoggerFactory;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Logger LOG = LoggerFactory.getLogger(HBaseReplicationEndpoint.class);
 
-  private ZKWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZKWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -112,7 +113,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -124,7 +127,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZKWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -132,10 +137,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZKWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZKWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -173,13 +182,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[7/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/9fbce166
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/9fbce166
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/9fbce166

Branch: refs/heads/master
Commit: 9fbce1668b1fc1e8a7d4bdb341bbed7bf65936f2
Parents: 3a80507
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:46:22 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/9fbce166/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index bd5c529..8286f7d 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.slf4j.LoggerFactory;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Logger LOG = LoggerFactory.getLogger(HBaseReplicationEndpoint.class);
 
-  private ZKWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZKWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -112,7 +113,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -124,7 +127,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZKWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -132,10 +137,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZKWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZKWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -173,13 +182,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);


[3/7] hbase git commit: HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint

Posted by ap...@apache.org.
HBASE-20597 Use a lock to serialize access to a shared reference to ZooKeeperWatcher in HBaseReplicationEndpoint


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/b50e1498
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/b50e1498
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/b50e1498

Branch: refs/heads/branch-1.3
Commit: b50e149804afeeb91bc561d2e2ae1ca6fe33c7d3
Parents: 94001b3
Author: Andrew Purtell <ap...@apache.org>
Authored: Thu May 17 10:30:28 2018 -0700
Committer: Andrew Purtell <ap...@apache.org>
Committed: Wed May 23 16:46:01 2018 -0700

----------------------------------------------------------------------
 .../replication/HBaseReplicationEndpoint.java   | 43 ++++++++++++--------
 1 file changed, 27 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/b50e1498/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
index 6485e4a..39a3f31 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/replication/HBaseReplicationEndpoint.java
@@ -43,21 +43,22 @@ import org.apache.zookeeper.KeeperException.SessionExpiredException;
  * target cluster is an HBase cluster.
  */
 @InterfaceAudience.Private
-@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="MT_CORRECTNESS",
-  justification="Thinks zkw needs to be synchronized access but should be fine as is.")
 public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   implements Abortable {
 
   private static final Log LOG = LogFactory.getLog(HBaseReplicationEndpoint.class);
 
-  private ZooKeeperWatcher zkw = null; // FindBugs: MT_CORRECTNESS
+  private Object zkwLock = new Object();
+  private ZooKeeperWatcher zkw = null;
 
   private List<ServerName> regionServers = new ArrayList<ServerName>(0);
   private long lastRegionServerUpdate;
 
   protected void disconnect() {
-    if (zkw != null) {
-      zkw.close();
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
     }
   }
 
@@ -102,7 +103,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
   public synchronized UUID getPeerUUID() {
     UUID peerUUID = null;
     try {
-      peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      synchronized (zkwLock) {
+        peerUUID = ZKClusterId.getUUIDForCluster(zkw);
+      }
     } catch (KeeperException ke) {
       reconnect(ke);
     }
@@ -114,7 +117,9 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @return zk connection
    */
   protected ZooKeeperWatcher getZkw() {
-    return zkw;
+    synchronized (zkwLock) {
+      return zkw;
+    }
   }
 
   /**
@@ -122,10 +127,14 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * @throws IOException If anything goes wrong connecting
    */
   void reloadZkWatcher() throws IOException {
-    if (zkw != null) zkw.close();
-    zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
+    synchronized (zkwLock) {
+      if (zkw != null) {
+        zkw.close();
+      }
+      zkw = new ZooKeeperWatcher(ctx.getConfiguration(),
         "connection to cluster: " + ctx.getPeerId(), this);
-    getZkw().registerListener(new PeerRegionServerListener(this));
+      zkw.registerListener(new PeerRegionServerListener(this));
+    }
   }
 
   @Override
@@ -163,13 +172,15 @@ public abstract class HBaseReplicationEndpoint extends BaseReplicationEndpoint
    * for this peer cluster
    * @return list of addresses
    */
-  // Synchronize peer cluster connection attempts to avoid races and rate
-  // limit connections when multiple replication sources try to connect to
-  // the peer cluster. If the peer cluster is down we can get out of control
-  // over time.
-  public synchronized List<ServerName> getRegionServers() {
+  public List<ServerName> getRegionServers() {
     try {
-      setRegionServers(fetchSlavesAddresses(this.getZkw()));
+      // Synchronize peer cluster connection attempts to avoid races and rate
+      // limit connections when multiple replication sources try to connect to
+      // the peer cluster. If the peer cluster is down we can get out of control
+      // over time.
+      synchronized (zkwLock) {
+        setRegionServers(fetchSlavesAddresses(zkw));
+      }
     } catch (KeeperException ke) {
       if (LOG.isDebugEnabled()) {
         LOG.debug("Fetch slaves addresses failed", ke);