Fwd: Modify ZooKeeper Java client to hold weak references to Watcher objects

[Dominic Williams <th...@googlemail.com>]

> I mistakenly sent this to the users list a few hours ago. My apologises if
you've seen it already. Here it is cleaned up a little!

Hi ZooKeepers!

The current ZooKeeper client holds strong references to Watcher objects. I
want to change the client so it only holds weak references. Feedback please.

*** Problem Example (contrived, but illustrates point!) ***
I have a game application with a large number of virtual "lobbies", each of
which has its own znode on ZooKeeper. Each lobby has a list of online
"coordinators" which is indicated by the ephemeral children of the znode. As
clients of the system move around the lobbies, they watch the relevant
coordinator list for their current lobby, so they can re-register with new
coordinators if they join. There are a huge number of clients, and clients
change lobbies frequently. So imagine:
- I create a ChangableSet sync primitive that clients use to watch the
coordinator list for their current lobby
- Each time a client changes lobby, it simply assigns a new coordinator list
e.g. something like "coordinatorList = new CoordinatorList extends
ChangableSet(IChangeListener listener) {}"
- The ChangableSet primitive is a Watcher object. Thus even when we
overwrite a reference to an instance when changing lobbies, it remains
pinned in memory: The ZooKeeper client keeps its own internal reference to
the Watcher until a new NodeChildrenChanged event occurs. In this case, new
coordinators join only rarely, so instances effectively stay there.
- As the client moves between lobbies, he leaves a trail of ChangeableSet
objects pinned in memory
- Huge numbers of clients moving between lobbies creates a memory problem,
and a massive event routing table inside the ZooKeeper client
---

*** Solutions that don't work ***
1. Add Watcher "de-registration" function to ZooKeeper client
The problem here is where client classes should be de-registered? In the
case of a locking primitive say, where it will always be unlocked in a
finally statement (at least in synchronous programming) we always have
somewhere to perform de-registration. But what about something like the
ChangeableSet primitive above - in this case users of the primitive would
have to call close() or similar on instances before losing a reference to
them. This is kind of inviting problems.
2. Have Watcher objects de-register themselves on finalize() (we have to
make it an abstract class, but just say!)
Of course this won't work because finalize() will never be called since
ZooKeeper maintains a reference to them! etc
3. Force an idiom where in this case, for example, the application maintains
a map of ChangeableSet objects, and each ChangeableSet class maintains a
cache of listeners... This does work, but I'd say we are pushing complexity
to the client application, so this should be avoided
---

*** Proposed WeakReference solution ***
Simply make the following change inside ZooKeeper.java
    private static class ZKWatchManager implements ClientWatchManager {
        private final Map<String, Set<Watcher>> dataWatches =
            new HashMap<String, Set<Watcher>>();
        private final Map<String, Set<Watcher>> existWatches =
            new HashMap<String, Set<Watcher>>();
        private final Map<String, Set<Watcher>> childWatches =
            new HashMap<String, Set<Watcher>>();
>>>
    private static class ZKWatchManager implements ClientWatchManager {
        private final Map<String, WeakSet<Watcher>> dataWatches =
            new HashMap<String, WeakSet <Watcher>>();
        private final Map<String, WeakSet <Watcher>> existWatches =
            new HashMap<String, WeakSet <Watcher>>();
        private final Map<String, WeakSet <Watcher>> childWatches =
            new HashMap<String, WeakSet <Watcher>>();
For example use the excellent WeakSet implementation written by Sun Engineer
Ales Novak in 2007 which can be found here
http://www.java2s.com/Code/Java/Collections-Data-Structure/SetwhichholdsitsmembersbyusingofWeakReferences.htm

Now, when there are no other references to a Watcher object, the ZooKeeper
client automatically de-registers it as a listener for events and the
Watcher object can be garbage collected.

Now clients can freely create new ChangeableSet objects as they move between
lobbies. Their old ChangeableSets will be deregistered from ZooKeeper
automatically (thus also keeping ZKWatchManager event routing table small
and efficient) and will be garbage collected as normal.

There are other benefits. For example, now we can implement a lock primitive
that unlocks itself in finalize(), preventing careless programmers from
creating deadlock (maybe when unlocking happens in finalize, some kind of
warning can be logged)
---

This all seems quite positive. If devs agree / I haven't missed something,
which is very possible (sorry if I have!), will make changes and perform
limited testing within my app then submit to Jira.

Best, Dominic Williams
http://ria101.wordpress.com