Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 3B9A01792B for ; Fri, 10 Oct 2014 17:29:26 +0000 (UTC) Received: (qmail 72729 invoked by uid 500); 10 Oct 2014 17:29:25 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 72686 invoked by uid 500); 10 Oct 2014 17:29:25 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 72675 invoked by uid 99); 10 Oct 2014 17:29:25 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 10 Oct 2014 17:29:25 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id A390E81907A; Fri, 10 Oct 2014 17:29:25 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: stack@apache.org To: commits@hbase.apache.org Message-Id: <8b3e87f22c4747a1af14e2ff0b18a6ca@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: HBASE-11394 Replication can have data loss if peer id contains hyphen "-" Date: Fri, 10 Oct 2014 17:29:25 +0000 (UTC) Repository: hbase Updated Branches: refs/heads/0.98 876617bd3 -> da0af93f8 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/da0af93f Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/da0af93f Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/da0af93f Branch: refs/heads/0.98 Commit: da0af93f80a42e23fef6593d3d1523656eddbfd6 Parents: 876617b Author: stack Authored: Fri Oct 10 10:28:16 2014 -0700 Committer: stack Committed: Fri Oct 10 10:29:16 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/da0af93f/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 21d8c1c..abdfd95 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 @@ -116,6 +116,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 listOfOps = new ArrayList(); ZKUtilOp op1 = http://git-wip-us.apache.org/repos/asf/hbase/blob/da0af93f/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 86d2a2c..1cb36a9 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 @@ -1324,7 +1324,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/da0af93f/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 d073726..c8e2ef2 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 @@ -141,7 +141,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", utility.getClusterKey()); rt.registerListener(new DummyReplicationListener()); @@ -153,7 +153,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", utility.getClusterKey()); @@ -170,9 +170,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