From eaafe0b0097ac7702faec27449748b2090b12c54 Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Wed, 18 Dec 2019 16:36:17 +0100 Subject: [PATCH 1/6] ZOOKEEPER-3414 sync api throws NoNodeException when path is non-existent --- .../src/main/java/org/apache/zookeeper/cli/SyncCommand.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java index 1c045757baf..9d10157f3ba 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java @@ -27,6 +27,7 @@ import org.apache.commons.cli.Parser; import org.apache.commons.cli.PosixParser; import org.apache.zookeeper.AsyncCallback; +import org.apache.zookeeper.KeeperException; /** * sync command for cli @@ -74,7 +75,7 @@ public void processResult(int rc, String path, Object ctx) { if (resultCode == 0) { out.println("Sync is OK"); } else { - out.println("Sync has failed. rc=" + resultCode); + throw new CliWrapperException(new KeeperException.NoNodeException(path)); } } catch (IllegalArgumentException ex) { throw new MalformedPathException(ex.getMessage()); From 1ca14668c8593f48387143306c4781093899d9e5 Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Fri, 20 Dec 2019 13:51:05 +0100 Subject: [PATCH 2/6] ZOOKEEPER-3414 Add catch block for nonode exception --- .../src/main/java/org/apache/zookeeper/cli/SyncCommand.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java index 9d10157f3ba..229e7a1e0f6 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java @@ -75,14 +75,14 @@ public void processResult(int rc, String path, Object ctx) { if (resultCode == 0) { out.println("Sync is OK"); } else { - throw new CliWrapperException(new KeeperException.NoNodeException(path)); + throw new KeeperException.NoNodeException(path); } } catch (IllegalArgumentException ex) { throw new MalformedPathException(ex.getMessage()); } catch (InterruptedException ie) { Thread.currentThread().interrupt(); throw new CliWrapperException(ie); - } catch (TimeoutException | ExecutionException ex) { + } catch (TimeoutException | ExecutionException | KeeperException.NoNodeException ex) { throw new CliWrapperException(ex); } From e995a6595a800e79e453e28efc25358ff623dc9c Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Thu, 2 Jan 2020 17:32:17 +0100 Subject: [PATCH 3/6] ZOOKEEPER-3414 Add exists call in sync with unit test --- .../java/org/apache/zookeeper/cli/SyncCommand.java | 13 +++++-------- .../java/org/apache/zookeeper/ZooKeeperTest.java | 12 ++++++++++++ 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java index 229e7a1e0f6..9f666c329d7 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java @@ -26,8 +26,8 @@ import org.apache.commons.cli.ParseException; import org.apache.commons.cli.Parser; import org.apache.commons.cli.PosixParser; -import org.apache.zookeeper.AsyncCallback; import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.data.Stat; /** * sync command for cli @@ -65,14 +65,11 @@ public boolean exec() throws CliException { CompletableFuture cf = new CompletableFuture<>(); try { - zk.sync(path, new AsyncCallback.VoidCallback() { - public void processResult(int rc, String path, Object ctx) { - cf.complete(rc); - } - }, null); + zk.sync(path, (rc, path1, ctx) -> cf.complete(rc), null); int resultCode = cf.get(SYNC_TIMEOUT, TimeUnit.MILLISECONDS); - if (resultCode == 0) { + Stat stat = zk.exists(path, false); + if (resultCode == 0 && stat != null) { out.println("Sync is OK"); } else { throw new KeeperException.NoNodeException(path); @@ -82,7 +79,7 @@ public void processResult(int rc, String path, Object ctx) { } catch (InterruptedException ie) { Thread.currentThread().interrupt(); throw new CliWrapperException(ie); - } catch (TimeoutException | ExecutionException | KeeperException.NoNodeException ex) { + } catch (TimeoutException | ExecutionException | KeeperException ex) { throw new CliWrapperException(ex); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java index d19f627e069..40c667ff603 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java @@ -691,4 +691,16 @@ public void testSyncCommand() throws Exception { runCommandExpect(cmd, expected); } + @Test(expected = CliWrapperException.class) + public void testSyncCommandFailure() throws Exception { + final ZooKeeper zk = createClient(); + SyncCommand cmd = new SyncCommand(); + cmd.setZk(zk); + cmd.parse("sync /dddd".split(" ")); + List expected = new ArrayList(); + expected.add("Sync should fail "); + + runCommandExpect(cmd, expected); + } + } From a7c59919e7f2283d03748883d73e82c770f1ccf3 Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Fri, 3 Jan 2020 16:02:08 +0100 Subject: [PATCH 4/6] ZOOKEEPER-3414 Use getData instead of exists modify test as well --- .../java/org/apache/zookeeper/cli/SyncCommand.java | 7 +++---- .../java/org/apache/zookeeper/ZooKeeperTest.java | 13 ++++++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java index 9f666c329d7..0f0090c1569 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java @@ -27,7 +27,6 @@ import org.apache.commons.cli.Parser; import org.apache.commons.cli.PosixParser; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.data.Stat; /** * sync command for cli @@ -68,11 +67,11 @@ public boolean exec() throws CliException { zk.sync(path, (rc, path1, ctx) -> cf.complete(rc), null); int resultCode = cf.get(SYNC_TIMEOUT, TimeUnit.MILLISECONDS); - Stat stat = zk.exists(path, false); - if (resultCode == 0 && stat != null) { + zk.getData(path, false, null); + if (resultCode == 0) { out.println("Sync is OK"); } else { - throw new KeeperException.NoNodeException(path); + out.println("Sync has failed. rc=" + resultCode); } } catch (IllegalArgumentException ex) { throw new MalformedPathException(ex.getMessage()); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java index 40c667ff603..2387a401fcc 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java @@ -691,16 +691,19 @@ public void testSyncCommand() throws Exception { runCommandExpect(cmd, expected); } - @Test(expected = CliWrapperException.class) + @Test public void testSyncCommandFailure() throws Exception { final ZooKeeper zk = createClient(); SyncCommand cmd = new SyncCommand(); cmd.setZk(zk); cmd.parse("sync /dddd".split(" ")); - List expected = new ArrayList(); - expected.add("Sync should fail "); - - runCommandExpect(cmd, expected); + try { + runCommandExpect(cmd, new ArrayList()); + fail("Path doesn't exist so, command should fail."); + } catch (CliWrapperException e) { + assertEquals(KeeperException.Code.NONODE, ((KeeperException) e.getCause()).code()); + assertEquals("Node does not exist: /dddd", e.getMessage()); + } } } From 838461c95d8bd41e1b71b7780b4a24b8f4a38777 Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Tue, 7 Jan 2020 14:23:35 +0100 Subject: [PATCH 5/6] ZOOKEEPER-3414 Improve testSyncCommandFailure --- .../src/test/java/org/apache/zookeeper/ZooKeeperTest.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java index 2387a401fcc..83d84c51c58 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java @@ -694,16 +694,15 @@ public void testSyncCommand() throws Exception { @Test public void testSyncCommandFailure() throws Exception { final ZooKeeper zk = createClient(); - SyncCommand cmd = new SyncCommand(); + final SyncCommand cmd = new SyncCommand(); cmd.setZk(zk); cmd.parse("sync /dddd".split(" ")); try { runCommandExpect(cmd, new ArrayList()); - fail("Path doesn't exist so, command should fail."); + fail("Command did not fail, even the path does not exist."); } catch (CliWrapperException e) { assertEquals(KeeperException.Code.NONODE, ((KeeperException) e.getCause()).code()); assertEquals("Node does not exist: /dddd", e.getMessage()); } } - } From 646c2a5b548ea8e3ff387c4493d0f7263d1892e3 Mon Sep 17 00:00:00 2001 From: ravowlga123 Date: Mon, 17 Feb 2020 18:17:59 +0100 Subject: [PATCH 6/6] ZOOKEEPER-3414 Made changes in server side FinalRequestProcessor --- .../main/java/org/apache/zookeeper/cli/SyncCommand.java | 3 ++- .../org/apache/zookeeper/server/FinalRequestProcessor.java | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java index 0f0090c1569..d904b298151 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java @@ -67,9 +67,10 @@ public boolean exec() throws CliException { zk.sync(path, (rc, path1, ctx) -> cf.complete(rc), null); int resultCode = cf.get(SYNC_TIMEOUT, TimeUnit.MILLISECONDS); - zk.getData(path, false, null); if (resultCode == 0) { out.println("Sync is OK"); + } else if (resultCode == KeeperException.Code.NONODE.intValue()) { + throw new KeeperException.NoNodeException(path); } else { out.println("Sync has failed. rc=" + resultCode); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java index 9ffde55c10c..643791b98f7 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java @@ -336,7 +336,12 @@ public void processRequest(Request request) { lastOp = "SYNC"; SyncRequest syncRequest = new SyncRequest(); ByteBufferInputStream.byteBuffer2Record(request.request, syncRequest); - rsp = new SyncResponse(syncRequest.getPath()); + path = syncRequest.getPath(); + DataNode n = zks.getZKDatabase().getNode(path); + if (n == null) { + throw new KeeperException.NoNodeException(); + } + rsp = new SyncResponse(path); requestPathMetricsCollector.registerRequest(request.type, syncRequest.getPath()); break; }