Skip to content

ZOOKEEPER-3414 sync api throws NoNodeException when path is non-existent #1187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
rabi-kumar wants to merge 7 commits into
base: master
Choose a base branch
from
+24 −2
Open

ZOOKEEPER-3414 sync api throws NoNodeException when path is non-existent #1187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
eaafe0b
ZOOKEEPER-3414 sync api throws NoNodeException when path is non-existent
rabi-kumar Dec 18, 2019
1ca1466
ZOOKEEPER-3414 Add catch block for nonode exception
rabi-kumar Dec 20, 2019
e995a65
ZOOKEEPER-3414 Add exists call in sync with unit test
rabi-kumar Jan 2, 2020
c5744db
ZOOKEEPER-3414 Resolve conflict
rabi-kumar Jan 2, 2020
a7c5991
ZOOKEEPER-3414 Use getData instead of exists modify test as well
rabi-kumar Jan 3, 2020
838461c
ZOOKEEPER-3414 Improve testSyncCommandFailure
rabi-kumar Jan 7, 2020
646c2a5
ZOOKEEPER-3414 Made changes in server side FinalRequestProcessor
rabi-kumar Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion zookeeper-server/src/main/java/org/apache/zookeeper/cli/SyncCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.commons.cli.ParseException;
import org.apache.commons.cli.Parser;
import org.apache.commons.cli.PosixParser;
import org.apache.zookeeper.KeeperException;

/**
* sync command for cli
Expand Down Expand Up @@ -68,6 +69,8 @@ public boolean exec() throws CliException {
int resultCode = cf.get(SYNC_TIMEOUT, TimeUnit.MILLISECONDS);
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);
}
Expand All @@ -76,7 +79,7 @@ public boolean exec() throws CliException {
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
throw new CliWrapperException(ie);
} catch (TimeoutException | ExecutionException ex) {
} catch (TimeoutException | ExecutionException | KeeperException ex) {
throw new CliWrapperException(ex);
}

Expand Down
7 changes: 6 additions & 1 deletion zookeeper-server/src/main/java/org/apache/zookeeper/server/FinalRequestProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
14 changes: 14 additions & 0 deletions zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,4 +691,18 @@ public void testSyncCommand() throws Exception {
runCommandExpect(cmd, expected);
}

@Test
public void testSyncCommandFailure() throws Exception {
final ZooKeeper zk = createClient();
final SyncCommand cmd = new SyncCommand();
cmd.setZk(zk);
cmd.parse("sync /dddd".split(" "));
try {
runCommandExpect(cmd, new ArrayList<String>());
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());
}
}
}