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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
