ctubbsii commented on code in PR #5183:
URL: https://github.com/apache/accumulo/pull/5183#discussion_r1885418433
##########
core/src/main/java/org/apache/accumulo/core/clientImpl/ClientContext.java:
##########
@@ -229,8 +229,38 @@ public ClientContext(SingletonReservation reservation,
ClientInfo info,
this.info = info;
this.hadoopConf = info.getHadoopConf();
zooReader = new ZooReader(info.getZooKeepers(),
info.getZooKeepersSessionTimeOut());
- zooCache =
- new ZooCacheFactory().getZooCache(info.getZooKeepers(),
info.getZooKeepersSessionTimeOut());
+
+ // Get the instanceID using ZooKeeper, not ZooCache as we will need
+ // the instanceID to create the ZooKeeper root path when creating
+ // ZooCache. If the instanceId cannot be found, this is a good
+ // time to find out.
Review Comment:
The chroot stuff that @meatballspaghetti is working on addresses this a
different way (very similar, actually, but as a prerequisite to the chroot
stuff, I've moved some of this code into ZooUtil, out of the ClientContext and
am further working to help improve that situation by simplifying the
bootstrapping of contexts by improving the way ClientInfo and ServerInfo
interact with ZooKeeper to initialize it).
I'm also getting rid of ZooCacheFactory and changing ZooCache to decorate a
ZooKeeper as part of my work to remove ZooSession in #5124, and these changes
conflict heavily with that.
I think this would be easier and simpler to do if this was held off until at
least #5124 is done, but possibly until some of the chroot stuff is done, too,
because it could have a significant impact on it.
Right now, I don't think we have an excessive number of watchers like we've
had in the past, so it's not much of a problem, and I think the benefits of
simplifying ZooSession and chroot our ZK interactions are going to be much more
beneficial.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
