ctubbsii commented on code in PR #5192:
URL: https://github.com/apache/accumulo/pull/5192#discussion_r1901318219
##########
core/src/main/java/org/apache/accumulo/core/fate/zookeeper/ZooUtil.java:
##########
@@ -116,7 +122,7 @@ public static String getRoot(final InstanceId instanceId) {
/**
* This method will delete a node and all its children.
*/
- public static void recursiveDelete(ZooKeeper zooKeeper, String zPath,
NodeMissingPolicy policy)
+ public static void recursiveDelete(ZooSession zooKeeper, String zPath,
NodeMissingPolicy policy)
Review Comment:
Good question. It looks like we created `ZooUtil.recursiveDelete` a very
long time ago to handle the case that ZooKeeper did not provide a recursive
delete implementation. Since then, ZooKeeper created a `ZKUtil.deleteRecursive`
utility, and then later overloaded the method to the ability to delete in
batches, which may be more performant. `ZooSession.ZKUtil` is just a proxy for
the real ZooKeeper `ZKUtil`, but which uses the `ZooSession`, but
`ZooUtil.recursiveDelete` is our own implementation. The main difference for
our purposes seems to be that the ZooUtil version supports customizing the
`NoNode` failure case.
As it relates to this PR, I didn't change the behavior of this at all. If
something called `ZooUtil.recursiveDelete` before, it still does... but passes
a `ZooSession` instead of a `ZooKeeper`. If something called ZooKeeper's
`ZKUtil.deleteRecursive` before with a `ZooKeeper`, it now calls
`ZooSession.ZKUtil.deleteRecursive` with a `ZooSession`. None of the behavior
changed.
I don't intend to do anything about this in this PR, but it may be worth a
follow-on to investigate if we no longer need `ZooUtil` and can call
ZooKeeper's `ZKUtil` from now on instead (via `ZooSession.ZKUtil` to pass
`ZooSession`)... or we can just add a recursive delete directly to ZooSession
(since its API doesn't actually have to match ZooKeeper exactly... it just is
easier to use if it's close).
--
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]
