Explore Moving ZooSession to ServerContext #2298
Assignees
Labels
enhancement
This issue describes a new feature, improvement, or optimization.
Milestone
Comments
milleruntime
added
the
enhancement
This was referenced Feb 24, 2022
|
I spent some time with this. It's hard to do because we use ZooSession in a few non-Context places, including in the legacy client code. This can probably be pushed to 3.0, where we can remove the legacy client code. |
ctubbsii
added a commit
to ctubbsii/accumulo
that referenced
this issue
Dec 23, 2024
* Replace existing ZooSession with a new one that is a facade for ZooKeeper. The methods from ZooKeeper we need are placed on it and internally it maintains a delegate ZooKeeper instance that handles automatically creating a new ZooKeeper session (client object) when the old session has died. This enables us to maintain fewer ZooKeeper objects by keeping only one at a time per ClientContext/ServerContext, and to reduce complexity substantially, where it was hard to reason about which ZooKeeper session we were using at any given moment. This no longer requires passing around ZooKeeper objects via ZooReader that called to the old ZooSession to construct a ZooKeeper session on demand. The new ZooSession is now a singleton field whose lifecycle is part of the context, and ZooReader/ZooReaderWriter are substantially simplified. * Lazily construct objects in ServerInfo/ClientInfoImpl to simplify the implementation for the various use cases, and to ensure that we don't create more objects than needed. * Improve debugging information to tie the ZooSession instances with their purpose (for example, for use with ServerContext, or for use with ClientContext, with the user's name) * Get rid of ZooCacheFactory in favor of a lazily constructed ZooCache instance in the ClientContext, to make it more clear when a ZooCache is being shared or reused, and to remove a static singleton * Move instanceId and instanceName lookup logic into ZooUtil * Make ZookeeperLockChecker use its own ZooSession, because it is still a static singleton and must continue to operate after the context that constructed it is closed (should be fixed when apache#2301 is done) * Perform some minor improvements to ZooCache to simplify its constructors now that it uses ZooSession, and to change the type of external watchers to Consumer, so they aren't as easily confused with actual ZooKeeper watchers * Improve a lot of ZooKeeper related test code Potential future work after this could include: * Roll ZooReader and ZooReaderWriter functions directly into ZooSession * Add support to ZooSession for more ZooKeeper APIs * Handle KeeperException thrown from delegate that signals the session is disconnected, instead of relying only on the verifyConnected() method in ZooSession to update the delegate * Handle InterruptedExceptions directly in ZooSession, so they don't propagate everywhere in the code in ways that are inconvenient to handle This fixes apache#5124, apache#2299, and apache#2298
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The class core/src/main/java/org/apache/accumulo/fate/zookeeper/ZooSession.java registers a static singleton with SingletonManager. It might be possible to refactor this code to live in ServerContext. There is already a
ZooReaderWriterobject living inServerContext.There are two access points to the static state in ZooSession:
The text was updated successfully, but these errors were encountered: