[KYUUBI #7556] Rearm the ZooKeeper engine deregistration watcher after reconnect - #7558
[KYUUBI #7556] Rearm the ZooKeeper engine deregistration watcher after reconnect#7558Yangxuhao123 wants to merge 1 commit into
Conversation
0a3eb05 to
33c7223
Compare
|
@Yangxuhao123 rebase on latest master can solve the CI timeout issue. cc @cxzl25 could you please take a look? |
…r after reconnect
33c7223 to
c9260b5
Compare
| " ZooKeeper. The server will be shut down after the last client session completes.") | ||
| ZookeeperDiscoveryClient.this.deregisterService() | ||
| serviceDiscovery.stopGracefully() | ||
| } else if (event.getType == Watcher.Event.EventType.NodeCreated) { |
There was a problem hiding this comment.
What version of zookeeper are you using?
There was a problem hiding this comment.
@cxzl25 zk client is 3.4.14, zk server is 3.8.4, Is this related to zookeeper version change?
There was a problem hiding this comment.
Sorry for the late reply.
We encountered several ZooKeeper-related issues.
- We previously attempted to re-register the watcher unconditionally within
DeRegisterWatcherwhenever any event was received. However, under certain circumstances, this produced adverse side effects and prevented ZooKeeper from reconnecting when exceptions occurred.
[TASK][EASY] DeRegisterWatcher should re-watch when any event is received #6477 - In the early phase, we faced unexpected closures of ZooKeeper sessions. After investigation, we found it was related to this ticket: https://issues.apache.org/jira/browse/ZOOKEEPER-1622
- We removed
ConnectionStateListenablein our internal build. Although Kyuubi supports self-recovery to theRECONNECTEDstate after entering theLOSTstate, Curator will not attempt to reconnect to ZooKeeper without any API invocations via zkclient.
Regarding the rewatch logic you plan to implement: is there a possibility that when you trigger watch re-registration, the server registers another identical engine instance?
There was a problem hiding this comment.
Sorry, I've been busy with some work-related matters recently.
Regarding the third point you mentioned, it is indeed a solution measure.
Regarding the issue you mentioned, we are currently conducting tests with the QA team.
|
@pan3793 Could you please review this ？ |
3 participants
Why are the changes needed?
Closes #7556.
ZooKeeper watches are one-shot. After a Kyuubi engine's ZooKeeper session expires and reconnects, its service node is restored, but the
DeRegisterWatcheris not automatically rearmed. A subsequent node deletion may therefore be missed, preventing graceful shutdown and allowing an unhealthy engine to remain discoverable.This patch rearms the watcher asynchronously after
RECONNECTED, retries when the service node is temporarily unavailable, handles node recreation, and invalidates stale or pending rewatch attempts after a newer reconnect or service deregistration.How was this patch tested?
Added embedded ZooKeeper test coverage for:
RECONNECTEDevent followed by node deletion.The following test suite was executed:
./build/mvn test -pl kyuubi-ha -am \ -Dtest=none \ -DwildcardSuites=org.apache.kyuubi.ha.client.zookeeper.EmbeddedZookeeperDiscoveryClientSuiteAll 16 tests passed.
Was this patch authored or co-authored using generative AI tooling?
Assisted-by: OpenAI Codex (GPT-5)