curator-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jordan Zimmerman <jor...@jordanzimmerman.com>
Subject Re: NamespaceWatcher hashCode and equals still bugging me
Date Tue, 09 Feb 2016 17:01:48 GMT
> So.... taking a step back, what was underlying motivation for the hashCode / equality
changes?  IE, what's the bigger problem we were trying to solve?

Before this change, we were maintaining a map from Watcher to NamespaceWatcher so that we
could track/remove the wrapped watcher. This is necessary due to this guarantee of ZooKeeper:

http://zookeeper.apache.org/doc/trunk/zookeeperProgrammers.html#sc_WatchGuarantees

"if the same watch object is registered for an exists and a getData call for the same file
and that file is then deleted, the watch object would only be invoked once with the deletion
notification for the file.”

Given that NamespaceWatcher is an internal wrapper, Curator needs to generate the same NamespaceWatcher
for a given client’s Watcher/CuratorWatcher. The map handled this. In the past, this was
difficult to manage and had potential memory leaks if the map wasn’t managed correctly.
It occurred to me that the map isn’t needed if NamespaceWatcher could have equality/hash
values the same as the Watcher that it wraps. My testing proved this. 

Thoughts?

-Jordan


> On Feb 9, 2016, at 11:49 AM, Scott Blum <dragonsinth@gmail.com> wrote:
> 
> Hi guys,
> 
> I'm a practical guy, not a purist, but the 3.0 implementations of NamespaceWatcher.hashCode()
and equals() are bothering me.  The reason I care is that I want to avoid subtle bugs cropping
up.
> 
> So here's the problem.
> 
> 1) equals() is not reflexive between NamespaceWatcher and Watcher
> 
> Assuming you have a NamespaceWatcher nw wrapping a Watcher w, the following code might
or might not work:
> 
> container.add(nw)
> container.remove(w)
> 
> It depends on whether the underlying container ultimately does "nw.equals(w)" or "w.equals(nw)".
 Set.contains() would have the same problem.
> 
> 2) hashCode() and equals() inconsistent with each other
> 
> Because nw.hashCode() != w.hashCode(), lookups in a hashSet or hashMap will practically
never work except by luck.
> 
> hashSet.put(nw)
> hashSet.contains(w)
> 
> Most of the time this will return false, except in the exact case where nw and w happen
to have hashCodes that map into the same bucket, and the equality check is done the "right"
order.
> 
> 
> So.... taking a step back, what was underlying motivation for the hashCode / equality
changes?  IE, what's the bigger problem we were trying to solve?
> 
> Scott
> 


Mime
View raw message