Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable ZooKeeper client to establish connection in read-only mode #4244

Merged
merged 1 commit into from
Jul 10, 2024

Conversation

massakam
Copy link
Contributor

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.

public Builder allowReadOnlyMode(boolean allowReadOnlyMode) {
this.allowReadOnlyMode = allowReadOnlyMode;
return this;
}

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.

switch (event.getState()) {
case SyncConnected:
LOG.info("ZooKeeper client is connected now.");
clientConnectLatch.countDown();
break;
case Disconnected:
LOG.info("ZooKeeper client is disconnected from zookeeper now,"
+ " but it is OK unless we received EXPIRED event.");
break;
case Expired:
clientConnectLatch = new CountDownLatch(1);
LOG.error("ZooKeeper client connection to the ZooKeeper server has expired!");
break;
default:
// do nothing
break;
}

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

@massakam
Copy link
Contributor Author

Ran git rebase master.

@massakam
Copy link
Contributor Author

massakam commented Jul 10, 2024

@shoothzj @dlg99 @hangc0276
All checks have passed so would you please merge this?

@hezhangjian hezhangjian merged commit 4d50a44 into apache:master Jul 10, 2024
23 checks passed
@massakam massakam deleted the allow-zk-ro-mode branch July 10, 2024 06:12
Ghatage pushed a commit to sijie/bookkeeper that referenced this pull request Jul 12, 2024
…ache#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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants