[GitHub] [accumulo] milleruntime commented on pull request #1812: Stop passing watcher to ZooCache exists()

[GitBox <gi...@apache.org>]

milleruntime commented on pull request #1812:
URL: https://github.com/apache/accumulo/pull/1812#issuecomment-735801360


   > This comment explains why the existence check is important. However, it's not just the existence check on its own... it's specifically the existence check _with_ the watcher, because a watcher passed to an `exists` method will register to watch for node creation events, while a watcher passed to a `getData` method will register to watch for node changed events. ZooCache needs both to work correctly.
   
   I partially agree with you.  I do not think that we need a watcher for everytime we call `exists()`.  I read the comment to mean that we need the `ZCacheWatcher` to be on the `exists()`, which I am not convinced is true anymore.  The `ZCacheWatcher` only ever removes from the cache.  Removal from the cache should probably be handled with a timeout like we do with the Guava caches.  This is a separate issue though.
   
   That being said, there may be other watchers being passed in that depend on the event trigger from the `exists()` method, which I believe is what you are saying.  Unfortunately this code is awful and its really hard to figure out which watchers depend on this event.  @EdColeman and I were hoping we could at least fix #1809 until it can be rewritten.
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org