Skip to content

Commit 4d50a44

Browse files
authored
Enable ZooKeeper client to establish connection in read-only mode (#4244)
### Motivation If the system property `readonlymode.enabled` is set to true on a ZooKeeper server, read-only mode is enabled. Data can be read from the server in read-only mode even if that server is split from the quorum. https://zookeeper.apache.org/doc/current/zookeeperAdmin.html#Experimental+Options%2FFeatures To connect to the server in read-only mode, the client must also allow read-only mode. The `ZooKeeperClient` class in the bookkeeper repository also has an option called `allowReadOnlyMode`. https://github.com/apache/bookkeeper/blob/15171e1904f7196d8e9f4116ab2aecdf582e0032/bookkeeper-server/src/main/java/org/apache/bookkeeper/zookeeper/ZooKeeperClient.java#L219-L222 However, even if this option is set to true, the connection to the server in read-only mode will actually fail. The cause is in the `ZooKeeperWatcherBase` class. When the `ZooKeeperWatcherBase` class receives the `SyncConnected` event, it releases `clientConnectLatch` and assumes that the connection is complete. https://github.com/apache/bookkeeper/blob/15171e1904f7196d8e9f4116ab2aecdf582e0032/bookkeeper-server/src/main/java/org/apache/bookkeeper/zookeeper/ZooKeeperWatcherBase.java#L128-L144 However, if the server is in read-only mode, it will receive `ConnectedReadOnly` instead of `SyncConnected`. This causes the connection to time out without being completed. ### Changes Modified the switch statement in the `ZooKeeperWatcherBase` class to release `clientConnectLatch` when `ConnectedReadOnly` is received if the `allowReadOnlyMode` option is true. By the way, `allowReadOnlyMode` is never set to true in BookKeeper. So this change would be useless for BookKeeper. However, it is useful for Pulsar. Because Pulsar also uses `ZooKeeperWatcherBase` and needs to be able to connect to ZooKeeper in read-only mode. https://github.com/apache/pulsar/blob/cba1600d0f6a82f1ea194f3214a80f283fe8dc27/pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/PulsarZooKeeperClient.java#L242-L244
1 parent 772b162 commit 4d50a44

File tree

8 files changed

+59
-12
lines changed

8 files changed

+59
-12
lines changed

bookkeeper-server/src/main/java/org/apache/bookkeeper/zookeeper/ZooKeeperClient.java

+3-3
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,9 @@ public ZooKeeperClient build() throws IOException, KeeperException, InterruptedE
241241

242242
// Create a watcher manager
243243
StatsLogger watcherStatsLogger = statsLogger.scope("watcher");
244-
ZooKeeperWatcherBase watcherManager =
245-
null == watchers ? new ZooKeeperWatcherBase(sessionTimeoutMs, watcherStatsLogger) :
246-
new ZooKeeperWatcherBase(sessionTimeoutMs, watchers, watcherStatsLogger);
244+
ZooKeeperWatcherBase watcherManager = (null == watchers)
245+
? new ZooKeeperWatcherBase(sessionTimeoutMs, allowReadOnlyMode, watcherStatsLogger)
246+
: new ZooKeeperWatcherBase(sessionTimeoutMs, allowReadOnlyMode, watchers, watcherStatsLogger);
247247
ZooKeeperClient client = new ZooKeeperClient(
248248
connectString,
249249
sessionTimeoutMs,

bookkeeper-server/src/main/java/org/apache/bookkeeper/zookeeper/ZooKeeperWatcherBase.java

+15-4
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public class ZooKeeperWatcherBase implements Watcher {
4444
.getLogger(ZooKeeperWatcherBase.class);
4545

4646
private final int zkSessionTimeOut;
47+
private final boolean allowReadOnlyMode;
4748
private volatile CountDownLatch clientConnectLatch = new CountDownLatch(1);
4849
private final CopyOnWriteArraySet<Watcher> childWatchers =
4950
new CopyOnWriteArraySet<Watcher>();
@@ -53,18 +54,20 @@ public class ZooKeeperWatcherBase implements Watcher {
5354
private final ConcurrentHashMap<EventType, Counter> eventCounters =
5455
new ConcurrentHashMap<EventType, Counter>();
5556

56-
public ZooKeeperWatcherBase(int zkSessionTimeOut) {
57-
this(zkSessionTimeOut, NullStatsLogger.INSTANCE);
57+
public ZooKeeperWatcherBase(int zkSessionTimeOut, boolean allowReadOnlyMode) {
58+
this(zkSessionTimeOut, allowReadOnlyMode, NullStatsLogger.INSTANCE);
5859
}
5960

60-
public ZooKeeperWatcherBase(int zkSessionTimeOut, StatsLogger statsLogger) {
61-
this(zkSessionTimeOut, new HashSet<Watcher>(), statsLogger);
61+
public ZooKeeperWatcherBase(int zkSessionTimeOut, boolean allowReadOnlyMode, StatsLogger statsLogger) {
62+
this(zkSessionTimeOut, allowReadOnlyMode, new HashSet<Watcher>(), statsLogger);
6263
}
6364

6465
public ZooKeeperWatcherBase(int zkSessionTimeOut,
66+
boolean allowReadOnlyMode,
6567
Set<Watcher> childWatchers,
6668
StatsLogger statsLogger) {
6769
this.zkSessionTimeOut = zkSessionTimeOut;
70+
this.allowReadOnlyMode = allowReadOnlyMode;
6871
this.childWatchers.addAll(childWatchers);
6972
this.statsLogger = statsLogger;
7073
}
@@ -130,6 +133,14 @@ public void process(WatchedEvent event) {
130133
LOG.info("ZooKeeper client is connected now.");
131134
clientConnectLatch.countDown();
132135
break;
136+
case ConnectedReadOnly:
137+
if (allowReadOnlyMode) {
138+
LOG.info("ZooKeeper client is connected in read-only mode now.");
139+
clientConnectLatch.countDown();
140+
} else {
141+
LOG.warn("ZooKeeper client is connected in read-only mode, which is not allowed.");
142+
}
143+
break;
133144
case Disconnected:
134145
LOG.info("ZooKeeper client is disconnected from zookeeper now,"
135146
+ " but it is OK unless we received EXPIRED event.");

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperClientZKSessionExpiry.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ public void run() {
5151
byte[] sessionPasswd = bkc.getZkHandle().getSessionPasswd();
5252

5353
try {
54-
ZooKeeperWatcherBase watcher = new ZooKeeperWatcherBase(10000);
54+
ZooKeeperWatcherBase watcher = new ZooKeeperWatcherBase(10000, false);
5555
ZooKeeper zk = new ZooKeeper(zkUtil.getZooKeeperConnectString(), 10000,
5656
watcher, sessionId, sessionPasswd);
5757
zk.close();

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookKeeperTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1072,7 +1072,7 @@ protected ZooKeeper createZooKeeper() throws IOException {
10721072
public void testZKConnectionLossForLedgerCreation() throws Exception {
10731073
int zkSessionTimeOut = 10000;
10741074
AtomicLong ledgerIdToInjectFailure = new AtomicLong(INVALID_LEDGERID);
1075-
ZooKeeperWatcherBase zooKeeperWatcherBase = new ZooKeeperWatcherBase(zkSessionTimeOut,
1075+
ZooKeeperWatcherBase zooKeeperWatcherBase = new ZooKeeperWatcherBase(zkSessionTimeOut, false,
10761076
NullStatsLogger.INSTANCE);
10771077
MockZooKeeperClient zkFaultInjectionWrapper = new MockZooKeeperClient(zkUtil.getZooKeeperConnectString(),
10781078
zkSessionTimeOut, zooKeeperWatcherBase, ledgerIdToInjectFailure);

bookkeeper-server/src/test/java/org/apache/bookkeeper/replication/TestReplicationWorker.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -1103,7 +1103,7 @@ public void testRWShutDownInTheCaseOfZKOperationFailures() throws Exception {
11031103
* create MockZooKeeperClient instance and wait for it to be connected.
11041104
*/
11051105
int zkSessionTimeOut = 10000;
1106-
ZooKeeperWatcherBase zooKeeperWatcherBase = new ZooKeeperWatcherBase(zkSessionTimeOut,
1106+
ZooKeeperWatcherBase zooKeeperWatcherBase = new ZooKeeperWatcherBase(zkSessionTimeOut, false,
11071107
NullStatsLogger.INSTANCE);
11081108
MockZooKeeperClient zkFaultInjectionWrapper = new MockZooKeeperClient(zkUtil.getZooKeeperConnectString(),
11091109
zkSessionTimeOut, zooKeeperWatcherBase);

bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ZooKeeperCluster.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ void sleepCluster(int time, TimeUnit timeUnit, CountDownLatch l)
6464
default void expireSession(ZooKeeper zk) throws Exception {
6565
long id = zk.getSessionId();
6666
byte[] password = zk.getSessionPasswd();
67-
ZooKeeperWatcherBase w = new ZooKeeperWatcherBase(10000);
67+
ZooKeeperWatcherBase w = new ZooKeeperWatcherBase(10000, false);
6868
ZooKeeper zk2 = new ZooKeeper(getZooKeeperConnectString(), zk.getSessionTimeout(), w, id, password);
6969
w.waitForConnection();
7070
zk2.close();

bookkeeper-server/src/test/java/org/apache/bookkeeper/test/ZooKeeperClusterUtil.java

+8
Original file line numberDiff line numberDiff line change
@@ -139,4 +139,12 @@ public void killCluster() throws Exception {
139139
public void sleepCluster(int time, TimeUnit timeUnit, CountDownLatch l) throws InterruptedException, IOException {
140140
throw new UnsupportedOperationException("sleepServer operation is not supported for ZooKeeperClusterUtil");
141141
}
142+
143+
public void stopPeer(int id) throws Exception {
144+
quorumUtil.shutdown(id);
145+
}
146+
147+
public void enableLocalSession(boolean localSessionEnabled) {
148+
quorumUtil.enableLocalSession(localSessionEnabled);
149+
}
142150
}

bookkeeper-server/src/test/java/org/apache/bookkeeper/zookeeper/TestZooKeeperClient.java

+29-1
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ public void process(WatchedEvent event) {
171171
};
172172
final int timeout = 2000;
173173
ZooKeeperWatcherBase watcherManager =
174-
new ZooKeeperWatcherBase(timeout).addChildWatcher(testWatcher);
174+
new ZooKeeperWatcherBase(timeout, false).addChildWatcher(testWatcher);
175175
List<Watcher> watchers = new ArrayList<Watcher>(1);
176176
watchers.add(testWatcher);
177177
ZooKeeperClient client = new ShutdownZkServerClient(
@@ -895,4 +895,32 @@ public void processResult(int rc, String path, Object ctx) {
895895
logger.info("Delete children from znode " + path);
896896
}
897897

898+
@Test
899+
public void testAllowReadOnlyMode() throws Exception {
900+
if (zkUtil instanceof ZooKeeperClusterUtil) {
901+
System.setProperty("readonlymode.enabled", "true");
902+
((ZooKeeperClusterUtil) zkUtil).enableLocalSession(true);
903+
zkUtil.restartCluster();
904+
Thread.sleep(2000);
905+
((ZooKeeperClusterUtil) zkUtil).stopPeer(2);
906+
((ZooKeeperClusterUtil) zkUtil).stopPeer(3);
907+
}
908+
909+
try (ZooKeeperClient client = ZooKeeperClient.newBuilder()
910+
.connectString(zkUtil.getZooKeeperConnectString())
911+
.sessionTimeoutMs(30000)
912+
.watchers(new HashSet<Watcher>())
913+
.operationRetryPolicy(retryPolicy)
914+
.allowReadOnlyMode(true)
915+
.build()) {
916+
Assert.assertTrue("Client failed to connect a ZooKeeper in read-only mode.",
917+
client.getState().isConnected());
918+
} finally {
919+
if (zkUtil instanceof ZooKeeperClusterUtil) {
920+
System.setProperty("readonlymode.enabled", "false");
921+
((ZooKeeperClusterUtil) zkUtil).enableLocalSession(false);
922+
}
923+
}
924+
}
925+
898926
}

0 commit comments

Comments
 (0)