alexguan / node-zookeeper-client

A pure Javascript ZooKeeper client for Node.js
Other
540 stars 147 forks source link

removeWatches support #50

Open bojanbass opened 8 years ago

bojanbass commented 8 years ago

Hi,

I was playing around with this library and also some other libs for zookeeper support in nodejs, but none has the option to unwatch some node after setting a watcher. From the official docs it looks like zookeeper is able to remove alreday set-up watchers: https://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#sc_WatchRemoval

I'm wondering if there's any option to do this?

alexguan commented 8 years ago

As soon as the watcher gets an event, it will be removed since each watcher only fired once. Could you let me know the reason behind removing watchers?

bojanbass commented 8 years ago

Thank you for the answer. I know watchers are one-time only. In my case I'm adding watchers on many different nodes based on users' UI widgets. There are many different filters per widget and also many different widget types possible. This means that there could be around 1000 watchers setup, but after a few hours only 10 are still relevant (this is just my guess, I don't have a real scenario yet). However, I read that zookeeper can easily operate with that many watchers, but I'm wondering if there could be memory leaks in nodejs code if it is still waiting for changes on those nodes and change never happens? That's why I was thinking about manual watcher removing.

leehambley commented 5 years ago

As soon as the watcher gets an event, it will be removed since each watcher only fired once. Could you let me know the reason behind removing watchers?

It would be great to add this to the documentation, my team and I just went looking for info on this (and found only this open issue).

Would you take a docs PR if I prepare one, or is it less work for you to do the doc change yourself?

alexguan commented 5 years ago

Yes, I do take PR for document updates. This behavior is well documented in zookeeper document, so I assumed most users know that already.

leehambley commented 5 years ago

This behavior is well documented in zookeeper document, so I assumed most users know that already.

We were able to confirm that ZK watchers fire once and only once, but we're not really expert in ZK and were just maintaining a pre-existing Node app that uses ZK and were worried that we were leaking watchers by never cleaning them up and always coming back through the same method never tidying up.