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 an application with some large number of virtual "lobbies", each of
which has its own znode on ZooKeeper. The "coordinators" of the rooms are
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 a large number of rooms, and clients
change rooms frequently:
- I create a ChangableSet sync primitive that triggers a callback when the
set (in this case a list of coordinators) changes
- Each time a client changes lobby, it simply does a "coordinatorList = new
CoordinatorList extends ChangableSet(IChangeListener listener) {}"
- The ChangableSet primitive is a watcher. This means the previously
assigned coordinatorList object will remain pinned in memory because the
client ZooKeeper keeps a reference to it as a Watcher (until the list of
coordinators changes, which happens only rarely)
- 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
Btw. Just because I'm developing a game, doesn't mean this example is real
:)
---
*** Discussion of discarded solutions ***
1. Add Watcher "de-registration" function to ZooKeeper client
The problem with this is where we should ask ZooKeeper client classes to
perform de-registration. In the case of a locking primitive say, it might be
straightforward (at least in synchronous programming) to demand something
like try { lock = new lock(); ... } finally { lock.unlock(); } in this case
de-registration always occurs. But what about a ChangeableSet primitive.
Should we be expecting clients in the lobbies example to
call coordinatorList .close() { zk.deregiserWatcher(this) } before
overwriting their ChangeableSet when they change lobby? We might demand this
as good practice, but there should not be a situation where memory leaks
occur when good practice is not followed
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 the application to maintain a cache of ChangeableSet objects, and
the ChangeableSet class to maintain a cache of listeners... This works, 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>>();
We 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 too. 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
