|
| 1 | +From 90e8e0f44e8a884765b6e7afe8bd779d59136fad Mon Sep 17 00:00:00 2001 |
| 2 | +From: Kezhu Wang < [email protected]> |
| 3 | +Date: Sat, 26 Apr 2025 12:04:01 +0800 |
| 4 | +Subject: ZOOKEEPER-4921: Retry endlessly to establish a brand-new session |
| 5 | + |
| 6 | +This partially rollback ZOOKEEPER-4508 to keep consistent with versions |
| 7 | +prior to 3.9.3 (excluded), so to maintain compatibility with third party |
| 8 | +libraries. |
| 9 | + |
| 10 | +Refs: ZOOKEEPER-4508, ZOOKEEPER-4921, ZOOKEEPER-4923 and |
| 11 | +https://lists.apache.org/thread/nfb9z7rhgglbjzfxvg4z2m3pks53b3c1 |
| 12 | +--- |
| 13 | + .../java/org/apache/zookeeper/ClientCnxn.java | 2 +- |
| 14 | + .../zookeeper/test/SessionTimeoutTest.java | 65 +++++++++++++------ |
| 15 | + 2 files changed, 47 insertions(+), 20 deletions(-) |
| 16 | + |
| 17 | +diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java |
| 18 | +index 0bf616c61..207bb8c49 100644 |
| 19 | +--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java |
| 20 | ++++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java |
| 21 | +@@ -1242,7 +1242,7 @@ public class ClientCnxn { |
| 22 | + to = connectTimeout - clientCnxnSocket.getIdleSend(); |
| 23 | + } |
| 24 | + |
| 25 | +- int expiration = expirationTimeout - clientCnxnSocket.getIdleRecv(); |
| 26 | ++ int expiration = sessionId == 0 ? Integer.MAX_VALUE : expirationTimeout - clientCnxnSocket.getIdleRecv(); |
| 27 | + if (expiration <= 0) { |
| 28 | + String warnInfo = String.format( |
| 29 | + "Client session timed out, have not heard from server in %dms for session id 0x%s", |
| 30 | +diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SessionTimeoutTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SessionTimeoutTest.java |
| 31 | +index 7a59f5eb9..9f5943f68 100644 |
| 32 | +--- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/SessionTimeoutTest.java |
| 33 | ++++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/SessionTimeoutTest.java |
| 34 | +@@ -18,6 +18,9 @@ |
| 35 | + |
| 36 | + package org.apache.zookeeper.test; |
| 37 | + |
| 38 | ++import static org.hamcrest.MatcherAssert.assertThat; |
| 39 | ++import static org.hamcrest.Matchers.greaterThanOrEqualTo; |
| 40 | ++import static org.hamcrest.Matchers.lessThan; |
| 41 | + import static org.junit.jupiter.api.Assertions.assertNotNull; |
| 42 | + import static org.junit.jupiter.api.Assertions.assertNull; |
| 43 | + import static org.junit.jupiter.api.Assertions.assertThrows; |
| 44 | +@@ -31,12 +34,15 @@ import java.util.List; |
| 45 | + import java.util.concurrent.CompletableFuture; |
| 46 | + import java.util.concurrent.CountDownLatch; |
| 47 | + import java.util.concurrent.TimeUnit; |
| 48 | ++import java.util.concurrent.TimeoutException; |
| 49 | + import org.apache.zookeeper.CreateMode; |
| 50 | + import org.apache.zookeeper.KeeperException; |
| 51 | + import org.apache.zookeeper.TestableZooKeeper; |
| 52 | + import org.apache.zookeeper.WatchedEvent; |
| 53 | + import org.apache.zookeeper.Watcher; |
| 54 | + import org.apache.zookeeper.ZooDefs; |
| 55 | ++import org.apache.zookeeper.ZooKeeper; |
| 56 | ++import org.apache.zookeeper.common.Time; |
| 57 | + import org.junit.jupiter.api.BeforeEach; |
| 58 | + import org.junit.jupiter.api.Test; |
| 59 | + import org.slf4j.Logger; |
| 60 | +@@ -54,6 +60,21 @@ public class SessionTimeoutTest extends ClientBase { |
| 61 | + zk = createClient(); |
| 62 | + } |
| 63 | + |
| 64 | ++ private static class ExpiredWatcher implements Watcher { |
| 65 | ++ public volatile CompletableFuture<Void> expired = new CompletableFuture<>(); |
| 66 | ++ |
| 67 | ++ synchronized void reset() { |
| 68 | ++ expired = new CompletableFuture<>(); |
| 69 | ++ } |
| 70 | ++ |
| 71 | ++ @Override |
| 72 | ++ public synchronized void process(WatchedEvent event) { |
| 73 | ++ if (event.getState() == Event.KeeperState.Expired) { |
| 74 | ++ expired.complete(null); |
| 75 | ++ } |
| 76 | ++ } |
| 77 | ++ } |
| 78 | ++ |
| 79 | + private static class BusyServer implements AutoCloseable { |
| 80 | + private final ServerSocket server; |
| 81 | + private final Socket client; |
| 82 | +@@ -143,17 +164,24 @@ public class SessionTimeoutTest extends ClientBase { |
| 83 | + // stop client also to gain less distraction |
| 84 | + zk.close(); |
| 85 | + |
| 86 | +- // small connection timeout to gain quick ci feedback |
| 87 | +- int sessionTimeout = 3000; |
| 88 | +- CompletableFuture<Void> expired = new CompletableFuture<>(); |
| 89 | ++ // given: established session |
| 90 | ++ int sessionTimeout = 3000; // small connection timeout to gain quick ci feedback |
| 91 | ++ ExpiredWatcher watcher = new ExpiredWatcher(); |
| 92 | + zk = createClient(new CountdownWatcher(), hostPort, sessionTimeout); |
| 93 | +- zk.register(event -> { |
| 94 | +- if (event.getState() == Watcher.Event.KeeperState.Expired) { |
| 95 | +- expired.complete(null); |
| 96 | +- } |
| 97 | +- }); |
| 98 | ++ zk.register(watcher); |
| 99 | ++ |
| 100 | ++ // when: all server down |
| 101 | ++ long start = Time.currentElapsedTime(); |
| 102 | ++ zk.sync("/"); // touch timeout counts |
| 103 | + stopServer(); |
| 104 | +- expired.join(); |
| 105 | ++ |
| 106 | ++ // then: get Expired after session timeout |
| 107 | ++ watcher.expired.join(); |
| 108 | ++ long elapsed = Time.currentElapsedTime() - start; |
| 109 | ++ assertThat(elapsed, greaterThanOrEqualTo((long) zk.getSessionTimeout())); |
| 110 | ++ assertThat(elapsed, lessThan(zk.getSessionTimeout() * 10L)); |
| 111 | ++ |
| 112 | ++ // then: future request will get SessionExpiredException |
| 113 | + assertThrows(KeeperException.SessionExpiredException.class, () -> zk.exists("/", null)); |
| 114 | + } |
| 115 | + |
| 116 | +@@ -162,18 +190,17 @@ public class SessionTimeoutTest extends ClientBase { |
| 117 | + // stop client also to gain less distraction |
| 118 | + zk.close(); |
| 119 | + |
| 120 | ++ // given: unavailable cluster |
| 121 | + stopServer(); |
| 122 | + |
| 123 | +- // small connection timeout to gain quick ci feedback |
| 124 | +- int sessionTimeout = 3000; |
| 125 | +- CompletableFuture<Void> expired = new CompletableFuture<>(); |
| 126 | +- new TestableZooKeeper(hostPort, sessionTimeout, event -> { |
| 127 | +- if (event.getState() == Watcher.Event.KeeperState.Expired) { |
| 128 | +- expired.complete(null); |
| 129 | +- } |
| 130 | +- }); |
| 131 | +- expired.join(); |
| 132 | +- assertThrows(KeeperException.SessionExpiredException.class, () -> zk.exists("/", null)); |
| 133 | ++ // when: try to establish a brand-new session |
| 134 | ++ int sessionTimeout = 300; // small connection timeout to gain quick ci feedback |
| 135 | ++ ExpiredWatcher watcher = new ExpiredWatcher(); |
| 136 | ++ try (ZooKeeper zk = new ZooKeeper(hostPort, sessionTimeout, watcher)) { |
| 137 | ++ // then: never Expired |
| 138 | ++ assertThrows(TimeoutException.class, () -> watcher.expired.get(3 * sessionTimeout, TimeUnit.MILLISECONDS)); |
| 139 | ++ assertThrows(KeeperException.ConnectionLossException.class, () -> zk.exists("/", null)); |
| 140 | ++ } |
| 141 | + } |
| 142 | + |
| 143 | + @Test |
0 commit comments