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-unsubscr...@zookeeper.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
