Repository: hbase Updated Branches: refs/heads/branch-1 a5b386412 -> 8c125a40b
HBASE-11394 Replication can have data loss if peer id contains hyphen "-" Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/8c125a40 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/8c125a40 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/8c125a40 Branch: refs/heads/branch-1 Commit: 8c125a40bf92b19e1062d058c653ef7d6e5200f4 Parents: a5b3864 Author: stack <[email protected]> Authored: Fri Oct 10 10:28:16 2014 -0700 Committer: stack <[email protected]> Committed: Fri Oct 10 10:28:44 2014 -0700 ---------------------------------------------------------------------- .../replication/ReplicationPeersZKImpl.java | 5 ++++ .../apache/hadoop/hbase/zookeeper/ZKUtil.java | 6 +++- .../TestReplicationTrackerZKImpl.java | 31 ++++++++++++++++++-- 3 files changed, 38 insertions(+), 4 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/8c125a40/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java index abb0984..cc49a64 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/replication/ReplicationPeersZKImpl.java @@ -110,6 +110,11 @@ public class ReplicationPeersZKImpl extends ReplicationStateZKBase implements Re throw new IllegalArgumentException("Cannot add a peer with id=" + id + " because that id already exists."); } + + if(id.contains("-")){ + throw new IllegalArgumentException("Found invalid peer name:" + id); + } + ZKUtil.createWithParents(this.zookeeper, this.peersZNode); List<ZKUtilOp> listOfOps = new ArrayList<ZKUtil.ZKUtilOp>(); ZKUtilOp op1 = ZKUtilOp.createAndFailSilent(ZKUtil.joinZNode(this.peersZNode, id), http://git-wip-us.apache.org/repos/asf/hbase/blob/8c125a40/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java index d3c39e5..eca72a8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKUtil.java @@ -1321,7 +1321,11 @@ public class ZKUtil { deleteNodeRecursively(zkw, joinZNode(node, child)); } } - zkw.getRecoverableZooKeeper().delete(node, -1); + //Zookeeper Watches are one time triggers; When children of parent nodes are deleted recursively. + //Must set another watch, get notified of delete node + if (zkw.getRecoverableZooKeeper().exists(node, zkw) != null){ + zkw.getRecoverableZooKeeper().delete(node, -1); + } } catch(InterruptedException ie) { zkw.interruptedException(ie); } http://git-wip-us.apache.org/repos/asf/hbase/blob/8c125a40/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/TestReplicationTrackerZKImpl.java ---------------------------------------------------------------------- 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 1c3de71..55de565 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 @@ -143,7 +143,7 @@ public class TestReplicationTrackerZKImpl { assertEquals("hostname2.example.org:1234", rsRemovedData); } - @Ignore ("Flakey") @Test(timeout = 30000) + @Test(timeout = 30000) public void testPeerRemovedEvent() throws Exception { rp.addPeer("5", new ReplicationPeerConfig().setClusterKey(utility.getClusterKey()), null); rt.registerListener(new DummyReplicationListener()); @@ -155,7 +155,7 @@ public class TestReplicationTrackerZKImpl { assertEquals("5", peerRemovedData); } - @Ignore ("Flakey") @Test(timeout = 30000) + @Test(timeout = 30000) public void testPeerListChangedEvent() throws Exception { // add a peer rp.addPeer("5", new ReplicationPeerConfig().setClusterKey(utility.getClusterKey()), null); @@ -172,9 +172,34 @@ public class TestReplicationTrackerZKImpl { assertTrue(plChangedData.contains("5")); // clean up - ZKUtil.deleteNode(zkw, "/hbase/replication/peers/5"); + //ZKUtil.deleteNode(zkw, "/hbase/replication/peers/5"); + rp.removePeer("5"); } + @Test(timeout = 30000) + public void testPeerNameControl() throws Exception { + int exists = 0; + int hyphen = 0; + rp.addPeer("6", new ReplicationPeerConfig().setClusterKey(utility.getClusterKey()), null); + + try{ + rp.addPeer("6", new ReplicationPeerConfig().setClusterKey(utility.getClusterKey()), null); + }catch(IllegalArgumentException e){ + exists++; + } + + try{ + rp.addPeer("6-ec2", new ReplicationPeerConfig().setClusterKey(utility.getClusterKey()), null); + }catch(IllegalArgumentException e){ + hyphen++; + } + assertEquals(1, exists); + assertEquals(1, hyphen); + + // clean up + rp.removePeer("6"); + } + private class DummyReplicationListener implements ReplicationListener { @Override
