[HBASE-12150] Backport replication changes from HBASE-12145

• Type: Task
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 0.98.7
• Hadoop Flags: Reviewed
• Assignee: Andrew Kyle Purtell
• Reporter: Andrew Kyle Purtell
• Created: 02/Oct/14 00:42
• Updated: 20/Nov/15 11:54
• Resolved: 03/Oct/14 01:15

Description

HBASE-12145 makes all zk accesses synchronized in RecoverableZooKeeper in branch-1 +:

@@ -690,23 +692,23 @@ public class RecoverableZooKeeper {
     return newData;
   }
 
-  public long getSessionId() {
-    return zk == null ? null : zk.getSessionId();
+  public synchronized long getSessionId() {
+    return zk == null ? -1 : zk.getSessionId();
   }
 
-  public void close() throws InterruptedException {
+  public synchronized void close() throws InterruptedException {
     if (zk != null) zk.close();
   }
 
-  public States getState() {
+  public synchronized States getState() {
     return zk == null ? null : zk.getState();
   }
 
-  public ZooKeeper getZooKeeper() {
+  public synchronized ZooKeeper getZooKeeper() {
     return zk;
   }
 
-  public byte[] getSessionPasswd() {
+  public synchronized byte[] getSessionPasswd() {
     return zk == null ? null : zk.getSessionPasswd();
   }

It also makes this change:

@@ -391,8 +390,14 @@ public class ReplicationPeersZKImpl extends ReplicationStateZKBase implements Re
     if (peer == null) {
       return false;
     }
-    ((ConcurrentMap<String, ReplicationPeerZKImpl>) peerClusters).putIfAbsent(peerId, peer);
-    LOG.info("Added new peer cluster " + peer.getPeerConfig().getClusterKey());
+    ReplicationPeerZKImpl previous =
+      ((ConcurrentMap<String, ReplicationPeerZKImpl>) peerClusters).putIfAbsent(peerId, peer);
+    if (previous == null) {
+      LOG.info("Added new peer cluster=" + peer.getPeerConfig().getClusterKey());
+    } else {
+      LOG.info("Peer already present, " + previous.getPeerConfig().getClusterKey() +
+        ", new cluster=" + peer.getPeerConfig().getClusterKey());
+    }
     return true;
   }

We should keep the 0.98 code in sync with these changes because these affect correctness. Would like to avoid "this change works in branch-1 or master but breaks in some weird way in 0.98" issues.