ctubbsii commented on code in PR #5350:
URL: https://github.com/apache/accumulo/pull/5350#discussion_r1990300218
##########
server/base/src/main/java/org/apache/accumulo/server/conf/store/impl/PropStoreWatcher.java:
##########
@@ -100,15 +103,15 @@ public void process(final WatchedEvent event) {
case NodeDataChanged:
path = event.getPath();
log.trace("handle change event for path: {}", path);
- propStoreKey = PropStoreKey.fromPath(path);
+ propStoreKey = PropStoreKey.fromPath(path, instanceId);
Review Comment:
I created an IT in 602fd4df7591971362373538df5c636d39b20bbc to verify the
paths are relative.
I also remember coming across a comment once in the ZK code that indicated
some paths might be absolute. I've been looking at the ZK code pretty
thoroughly, and I can't find it again. However, after looking at the ZK client
code, and seeing all the places where they convert the client-side view of the
path (relative to the chroot) to the server-side view of the path (the full
path) and back again, and after verifying this integration test works in 3.6.1
and 3.9.2, I'm pretty confident that wherever that comment was, it was either
wrong or did not apply to anything we care much about. I suspect that the
comment might have been tied to a server-side logged message, or an Exception
that got serialized with a path, because the chroot feature is implemented
fully on the client side, so anything the server spits out... possibly
including any KeeperException objects that are serialized and sent back to the
client... might still have the full path. I checked our code, and I don't see
any
critical use of KeeperException.getPath (just log messages).
Additionally, the latest documentation at
https://zookeeper.apache.org/doc/current/zookeeperProgrammers.html really
emphasizes the ability to write relocatable code for different applications,
chrooted to different parts of ZooKeeper. It would be really hard to do that if
they didn't have guarantees. In any case, my integration test should help avoid
the need to rely on confusing docs... it proves the current behavior and guards
against any obvious change in behavior with respect to watchers and paths.
--
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]
