dlmarion commented on PR #5183:
URL: https://github.com/apache/accumulo/pull/5183#issuecomment-2546197989
> Given how much this conflicts with #4809 and #5124, I'd like to hold off
on some of these, and would prefer to go with a much more narrow change that
just reuses the existing ZooCache on the context.
Re-using the ZooCache on the context is actually the larger change
code-wise. Modifying ZooCache to use the persistent recursive watchers is
pretty much self contained within ZooCache except for some tests.
> The prerequisite branch I made to help @meatballspaghetti get started on
the chroot stuff for #4809 and the work I have already nearly wrapped up for
#5124 are heavily conflicted with these changes, and I'd prefer to get those in
first.
>
I have no issue with waiting for #4809 and #5124 to be merged first,
assuming they are done relatively soon.
> Additionally, this PR seems to be broken on its reliance on a newer
ZooKeeper version, newer than the one we have been testing as a supported. Do
you know which ZK version introduced these features? If the versions required
to support this change are too narrow, it might really limit the deployment
options for users, if that range of versions is buggy in some way. In the past,
we've generally supported a pretty large range of ZooKeeper versions, allowing
quite a lot of flexibility for users when making upgrade decisions. I'd like to
not narrow the versions of ZK we support too much, if this was only introduced
in 3.9 or maybe even 3.8. But if this was introduced in 3.6 or 3.7, it's
probably fine.
Persistent Recursive Watchers were added in 3.6.0, so it's a very minor bump
in the minimum required version.
> I also have questions about how these new watchers work, and whether it'd
actually perform better. It seems there were some open questions about that.
I agree, which is why I targeted this for 4.0, to give us time to test 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
I don't know what excessive means in this context. The WatchTheWatchCountIT
is failing due to the increased watchers added in a recent PR for caching the
non-existent nodes in ZooCache.
--
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]
