You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@zookeeper.apache.org by "luozongle01 (via GitHub)" <gi...@apache.org> on 2025/03/18 15:52:24 UTC
[PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
luozongle01 opened a new pull request, #2237:
URL: https://github.com/apache/zookeeper/pull/2237
Modify client wait time: https://issues.apache.org/jira/browse/ZOOKEEPER-4909
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "luozongle01 (via GitHub)" <gi...@apache.org>.
luozongle01 commented on PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#issuecomment-2746049230
Hi @kezhuw , I have completed the modification, can you take a look at it again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "luozongle01 (via GitHub)" <gi...@apache.org>.
luozongle01 commented on PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#issuecomment-2745980687
Hi @kezhuw , I've made some changes. Could you help me check it again?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Fix exceeded request timeout in case of spurious wakeup [zookeeper]
Posted by "cnauroth (via GitHub)" <gi...@apache.org>.
cnauroth closed pull request #2237: ZOOKEEPER-4909: Fix exceeded request timeout in case of spurious wakeup
URL: https://github.com/apache/zookeeper/pull/2237
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "luozongle01 (via GitHub)" <gi...@apache.org>.
luozongle01 commented on code in PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#discussion_r2008993866
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ mt[i] = new MainThread(i, clientPorts[i], currentQuorumCfgSection, false);
+ mt[i].start();
+ }
+
+ // ensure server started
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPorts[i], CONNECTION_TIMEOUT),
+ "waiting for server " + i + " being up");
+ }
+
+ CountdownWatcher watch1 = new CountdownWatcher();
+ CustomZooKeeper zk = new CustomZooKeeper(getCxnString(clientPorts), ClientBase.CONNECTION_TIMEOUT, watch1);
+ watch1.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+
+ ReplyHeader r = new ReplyHeader();
+ ClientCnxn.Packet packet = new ClientCnxn.Packet(null, null, null, null, null);
+ ClientCnxn clientCnxn = zk.getClientCnxn();
+
+ // Simulate spurious wakeup.
+ new Thread(() -> {
+ try {
+ TimeUnit.MILLISECONDS.sleep(1500);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ synchronized (packet) {
+ packet.notifyAll();
+ }
+ }).start();
+
+ long startTime = System.currentTimeMillis();
Review Comment:
Yes, I neglected it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "kezhuw (via GitHub)" <gi...@apache.org>.
kezhuw commented on code in PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#discussion_r2008796107
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
Review Comment:
Do we need a cluster for this test ? Is a standalone server enough ?
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ mt[i] = new MainThread(i, clientPorts[i], currentQuorumCfgSection, false);
+ mt[i].start();
+ }
+
+ // ensure server started
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPorts[i], CONNECTION_TIMEOUT),
+ "waiting for server " + i + " being up");
+ }
+
+ CountdownWatcher watch1 = new CountdownWatcher();
+ CustomZooKeeper zk = new CustomZooKeeper(getCxnString(clientPorts), ClientBase.CONNECTION_TIMEOUT, watch1);
+ watch1.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+
+ ReplyHeader r = new ReplyHeader();
+ ClientCnxn.Packet packet = new ClientCnxn.Packet(null, null, null, null, null);
+ ClientCnxn clientCnxn = zk.getClientCnxn();
+
+ // Simulate spurious wakeup.
+ new Thread(() -> {
+ try {
+ TimeUnit.MILLISECONDS.sleep(1500);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ synchronized (packet) {
+ packet.notifyAll();
+ }
+ }).start();
+
+ long startTime = System.currentTimeMillis();
Review Comment:
Use `Time.currentElapsedTime` here also ?
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ mt[i] = new MainThread(i, clientPorts[i], currentQuorumCfgSection, false);
+ mt[i].start();
+ }
+
+ // ensure server started
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPorts[i], CONNECTION_TIMEOUT),
+ "waiting for server " + i + " being up");
+ }
+
+ CountdownWatcher watch1 = new CountdownWatcher();
+ CustomZooKeeper zk = new CustomZooKeeper(getCxnString(clientPorts), ClientBase.CONNECTION_TIMEOUT, watch1);
+ watch1.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+
+ ReplyHeader r = new ReplyHeader();
+ ClientCnxn.Packet packet = new ClientCnxn.Packet(null, null, null, null, null);
+ ClientCnxn clientCnxn = zk.getClientCnxn();
+
+ // Simulate spurious wakeup.
+ new Thread(() -> {
+ try {
+ TimeUnit.MILLISECONDS.sleep(1500);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ synchronized (packet) {
+ packet.notifyAll();
+ }
+ }).start();
+
+ long startTime = System.currentTimeMillis();
+ synchronized (packet) {
+ invokeMethod(clientCnxn, "waitForPacketFinish", r, packet);
Review Comment:
Is it possible to hang somewhere to simulate request timeout with possible large session timeout value instead of intercept this internal method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "luozongle01 (via GitHub)" <gi...@apache.org>.
luozongle01 commented on code in PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#discussion_r2008994000
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ mt[i] = new MainThread(i, clientPorts[i], currentQuorumCfgSection, false);
+ mt[i].start();
+ }
+
+ // ensure server started
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPorts[i], CONNECTION_TIMEOUT),
+ "waiting for server " + i + " being up");
+ }
+
+ CountdownWatcher watch1 = new CountdownWatcher();
+ CustomZooKeeper zk = new CustomZooKeeper(getCxnString(clientPorts), ClientBase.CONNECTION_TIMEOUT, watch1);
+ watch1.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+
+ ReplyHeader r = new ReplyHeader();
+ ClientCnxn.Packet packet = new ClientCnxn.Packet(null, null, null, null, null);
+ ClientCnxn clientCnxn = zk.getClientCnxn();
+
+ // Simulate spurious wakeup.
+ new Thread(() -> {
+ try {
+ TimeUnit.MILLISECONDS.sleep(1500);
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ synchronized (packet) {
+ packet.notifyAll();
+ }
+ }).start();
+
+ long startTime = System.currentTimeMillis();
+ synchronized (packet) {
+ invokeMethod(clientCnxn, "waitForPacketFinish", r, packet);
Review Comment:
Makes sense, I'll edit it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "luozongle01 (via GitHub)" <gi...@apache.org>.
luozongle01 commented on code in PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#discussion_r2008993898
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +99,90 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutSpuriousWakeup() throws Exception {
+ Long requestTimeout = 2000L;
+ System.setProperty("zookeeper.request.timeout", requestTimeout.toString());
+ final int[] clientPorts = new int[SERVER_COUNT];
+ StringBuilder sb = new StringBuilder();
+ String server;
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
+ clientPorts[i] = PortAssignment.unique();
+ server = "server." + i + "=127.0.0.1:" + PortAssignment.unique() + ":" + PortAssignment.unique()
+ + ":participant;127.0.0.1:" + clientPorts[i];
+ sb.append(server + "\n");
+ }
+ String currentQuorumCfgSection = sb.toString();
+ MainThread[] mt = new MainThread[SERVER_COUNT];
+
+ for (int i = 0; i < SERVER_COUNT; i++) {
Review Comment:
Yeah, I'll edit this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] ZOOKEEPER-4909: Modify client wait time. [zookeeper]
Posted by "kezhuw (via GitHub)" <gi...@apache.org>.
kezhuw commented on code in PR #2237:
URL: https://github.com/apache/zookeeper/pull/2237#discussion_r2009001042
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +102,105 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutTime() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
+ LOG.info("testClientRequestTimeoutTime cost:{}", cost);
+ assertTrue(cost >= requestTimeout);
+ assertTrue(cost < requestTimeout + 500);
+ }
+ } finally {
+ mainThread.shutdown();
+ if (zk != null) {
+ zk.close();
+ }
+ }
+ }
+
+
+ @Test
+ void testClientRequestTimeoutTimeSimulatingSpuriousWakeup() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+ capturePacket = true;
+ capturePacketType = ZooDefs.OpCode.create;
+
+ // Simulating spurious wakeup
+ new Thread(() -> {
+ try {
+ TimeUnit.SECONDS.sleep(4);
+ if (capturedPacket != null) {
+ synchronized (capturedPacket) {
+ capturedPacket.notifyAll();
+ }
+ }
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }).start();
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
+ LOG.info("testClientRequestTimeoutTimeSimulatingSpuriousWakeup cost:{}", cost);
+ assertTrue(cost >= requestTimeout);
+ assertTrue(cost < requestTimeout + 500);
Review Comment:
```suggestion
assertThat(cost, greaterThanOrEqualTo(requestTimeout));
assertThat(cost, lessThan(requestTimeout + 500));
```
Same as above.
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +102,105 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutTime() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
+ LOG.info("testClientRequestTimeoutTime cost:{}", cost);
+ assertTrue(cost >= requestTimeout);
+ assertTrue(cost < requestTimeout + 500);
Review Comment:
```suggestion
assertThat(cost, greaterThanOrEqualTo(requestTimeout));
assertThat(cost, lessThan(requestTimeout + 500));
```
In rare(a.k.a. flaky or debug) cases, it gives us more comparing to `assertEquals`.
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +102,105 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutTime() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
Review Comment:
```suggestion
assertEquals(KeeperException.Code.REQUESTTIMEOUT, exception.code());
```
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +102,105 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutTime() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
+ LOG.info("testClientRequestTimeoutTime cost:{}", cost);
+ assertTrue(cost >= requestTimeout);
+ assertTrue(cost < requestTimeout + 500);
+ }
+ } finally {
+ mainThread.shutdown();
+ if (zk != null) {
+ zk.close();
+ }
+ }
+ }
+
+
+ @Test
+ void testClientRequestTimeoutTimeSimulatingSpuriousWakeup() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+ capturePacket = true;
+ capturePacketType = ZooDefs.OpCode.create;
+
+ // Simulating spurious wakeup
+ new Thread(() -> {
+ try {
+ TimeUnit.SECONDS.sleep(4);
Review Comment:
```suggestion
TimeUnit.MILLISECONDS.sleep(requestTimeout / 2);
```
Make the bound to `requestTimeout` explicit.
##########
zookeeper-server/src/test/java/org/apache/zookeeper/ClientRequestTimeoutTest.java:
##########
@@ -94,6 +102,105 @@ public void testClientRequestTimeout() throws Exception {
}
}
+ @Test
+ void testClientRequestTimeoutTime() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
+ LOG.info("testClientRequestTimeoutTime cost:{}", cost);
+ assertTrue(cost >= requestTimeout);
+ assertTrue(cost < requestTimeout + 500);
+ }
+ } finally {
+ mainThread.shutdown();
+ if (zk != null) {
+ zk.close();
+ }
+ }
+ }
+
+
+ @Test
+ void testClientRequestTimeoutTimeSimulatingSpuriousWakeup() throws Exception {
+ long requestTimeout = TimeUnit.SECONDS.toMillis(5);
+ System.setProperty("zookeeper.request.timeout", Long.toString(requestTimeout));
+
+ CustomZooKeeper zk = null;
+ int clientPort = PortAssignment.unique();
+ MainThread mainThread = new MainThread(0, clientPort, "", false);
+ mainThread.start();
+ try {
+ assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPort, CONNECTION_TIMEOUT),
+ "waiting for server 0 being up");
+
+ CountdownWatcher watch = new CountdownWatcher();
+ zk = new CustomZooKeeper(getCxnString(new int[]{clientPort}), ClientBase.CONNECTION_TIMEOUT, watch);
+ watch.waitForConnected(ClientBase.CONNECTION_TIMEOUT);
+
+ dropPacket = true;
+ dropPacketType = ZooDefs.OpCode.create;
+ capturePacket = true;
+ capturePacketType = ZooDefs.OpCode.create;
+
+ // Simulating spurious wakeup
+ new Thread(() -> {
+ try {
+ TimeUnit.SECONDS.sleep(4);
+ if (capturedPacket != null) {
+ synchronized (capturedPacket) {
+ capturedPacket.notifyAll();
+ }
+ }
+ } catch (InterruptedException e) {
+ throw new RuntimeException(e);
+ }
+ }).start();
+
+ String data = "originalData";
+ long startTime = Time.currentElapsedTime();
+ try {
+ zk.create("/testClientRequestTimeout", data.getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT_SEQUENTIAL);
+ fail("KeeperException is expected.");
+ } catch (KeeperException exception) {
+ long cost = Time.currentElapsedTime() - startTime;
+ assertEquals(KeeperException.Code.REQUESTTIMEOUT.intValue(), exception.code().intValue());
Review Comment:
```suggestion
assertEquals(KeeperException.Code.REQUESTTIMEOUT, exception.code());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org