Hi all, Any thoughts on this?
Curator has similar issue reported as CURATOR-666[1]. I think it might be worth to get Curator and ZooKeeper behave same in `getConfig`. I replied this also to dev@curator for joint discussion. See my quotes and ZOOKEEPER-4565[2], ZOOKEEPER-4601[3] for context. [1]: https://issues.apache.org/jira/browse/CURATOR-666 [2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4565 [3]: https://issues.apache.org/jira/browse/ZOOKEEPER-4601 On Wed, Jul 27, 2022 at 8:35 PM Kezhu Wang <kez...@gmail.com> wrote: > Hi Devs, > > Before ZOOKEEPER-4565[1], `ClientCnxn` uses following code to strip chroot: > > ``` > // convert from a server path to a client path > if (chrootPath != null) { > String serverPath = event.getPath(); > if (serverPath.compareTo(chrootPath) == 0) { > event.setPath("/"); > } else if (serverPath.length() > chrootPath.length()) { > event.setPath(serverPath.substring(chrootPath.length())); > } else { > LOG.warn("Got server path {} which is too short for chroot path > {}.", > event.getPath(), chrootPath); > } > } > ``` > > This results in behavior: > * For chroot "/zookeeper", watcher will receive event path "/config". > * For chroot "/short", watcher will receive illegal path "eper/config". > This causes in ZOOKEEPER-4601. > * For chroot "/pretty-long-chroot-path", watcher will receive event path > "/zookeeper/config". > > ZOOKEEPEER-4601 changed the stripping code to fix illegal path: > > ``` > private String stripChroot(String serverPath) { > if (serverPath.startsWith(chrootPath)) { > if (serverPath.length() == chrootPath.length()) { > return "/"; > } > return serverPath.substring(chrootPath.length()); > } else if (serverPath.startsWith(ZooDefs.ZOOKEEPER_NODE_SUBTREE)) { > return serverPath; > } > LOG.warn("Got server path {} which is not descendant of chroot path > {}.", serverPath, chrootPath); > return serverPath; > } > ``` > > This results in behavior: > * For chroot "/zookeeper", watcher will receive event path "/config". > * For chroot "/other-chroot"(eg. "/short", "/pretty-long-chroot-path", > etc.), watcher will receive event path "/zookeeper/config". > > The path `getConfig` watcher received was not changed in ZOOKEEPER-4565. > > It is a bit of surprising to me that event path of `getConfig` watcher is > not "/zookeeper/config" . I guess the current behavior might not be by > design. Personally, I prefer to `getConfig` watchers(whether it is the > default one or not) to receive path "/zookeeper/config". But, obviously, > such change is a breaking change in behavior(though might not be by design) > of public API. @eolivelli mentioned that such a change needs a mailing list > decision, so I post it here for discussion in addition to ZOOKEEPER-4601[2]. > > Any thoughts ? > > [1]: https://issues.apache.org/jira/browse/ZOOKEEPER-4565 > [2]: https://issues.apache.org/jira/browse/ZOOKEEPER-4601 > > > Best, > Kezhu Wang > -- Best, Kezhu Wang
