MultiMap remove(key, value) does not work

[Jason Clawson]

Here is a simple example basically taken from the docs.  The expected output is:

true

true

true

3

What I get is:

false

false

false

6

MultiMap.remove(key,value) doesn't work :-(

import java.io.Serializable;

import java.util.Collection;

import com.hazelcast.core.Hazelcast;

import com.hazelcast.core.MultiMap;

public class MultimapBug {

public static void main(String[] args) {

MultiMap<Integer, Order> multimap = Hazelcast.getMultiMap("blah");

multimap.put(1, new Order(1));

multimap.put(1, new Order(2));

multimap.put(1, new Order(3));

multimap.put(2, new Order(4));

multimap.put(2, new Order(5));

multimap.put(2, new Order(6));

Collection<Order> items = multimap.get(1);

for (Order o : items) {

System.out.println(multimap.remove(1, o));

}

System.out.println("Size: " + multimap.size());

}

public static class Order implements Serializable {

private static final long serialVersionUID = 1L;

private int id;

public Order(int id) {

this.id = id;

}

}

}

[Tim Peierls]

You'll need to give your Order class sensible equals and hashCode methods. Bear in mind that MultiMap (like IMap and all the other distributed data structures in Hazelcast) stores serialized versions of its keys and values, so in general you can't rely on default object identity for removing or testing membership.

--tim

--
You received this message because you are subscribed to the Google Groups "Hazelcast" group.
To view this discussion on the web visit https://groups.google.com/d/msg/hazelcast/-/p7WDN7jOBvUJ.
To post to this group, send email to haze...@googlegroups.com.
To unsubscribe from this group, send email to hazelcast+...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/hazelcast?hl=en.

[Jason Clawson]

Yes, Hazelcast stores serialized version of keys and values.  This is exactly the reason why I assumed hashCode & equals was not needed here-- since they won't work in any other case because hazelcast compares the serialized, binary representation to determine quality.  It seems MultiMap is the only exception to this rule.

This is pretty confusing since some operations on multimap respect equals and hashCode while some do not.  For example, containsValue does not rely on equals and hashCode.  In my example above, if you replace:

System.out.println(multimap.remove(1, o)); 

with

System.out.println(multimap.contains(o)); 

it will return true for all 3 orders.

In all other cases equals and hashCode are meaningless to Hazelcast collections.  Why does it rely on them here?  Is a MultiMap implemented in such a way that.... multimap.put(1, new Order(1)) .... requires:

1. that hazelcast lock key "1"
2. download the entire HashSet for "1"
3. add the Order to the set
4. serialize the entire set again
5. store the new serialized set to "1"
6. then unlock "1"
   

?

I hope it doesn't work this way :-(.  If it doesn't, then remove(key, value) should really be optimized to not download the entire HashSet in order to remove a value.  Then Hazelcast would rely on the serialized form for identity and my code would work.

Thanks,

Jason

To unsubscribe from this group, send email to hazelcast+unsubscribe@googlegroups.com.

[Tim Peierls]

Is it a bug that contains and remove are inconsistent when equals and hashCode aren't defined? Maybe, but I claim that it's an error, in general, not to override equals and hashCode for types used as keys (i.e., targets of get/contains/containsKey/remove/etc.)

Hazelcast intentionally (and necessarily) violates the standard collection/map/multi-map contracts, as you already know, so while it is necessary to define equals and hashCode, it is not sufficient; you have to ensure that these methods are consistent with byte-for-byte equality of your serialized form. I have a general technique for doing this that works pretty well: I make all my Hazelcast-ready types extend a common base that implements DataSerializable using the Jackson binary Smile format. That common base overrides equals and hashCode in terms of the serialized form. Subtypes are free to specialize equals and hashCode further to get better performance where needed, as long as consistency between the overridden methods and the common base methods is maintained.

--tim

To view this discussion on the web visit https://groups.google.com/d/msg/hazelcast/-/WznQ7sjJBwoJ.

To post to this group, send email to haze...@googlegroups.com.

To unsubscribe from this group, send email to hazelcast+...@googlegroups.com.

[Mehmet Dogan]

Our general intention about using equals and hashCode is, 

• For keys; not to use equals and hashCode of key object; instead to use binary (serialized) form's equals and hashCode
• For values those require comparison (containsValue(v), replace(k, v1, v2), remove(k ,v) etc);
  to use original equals and hashCode.

We have tried to document those in javadocs of IMap and MultiMap. 

http://hazelcast.com/docs/2.1/javadoc/com/hazelcast/core/IMap.html

http://hazelcast.com/docs/2.1/javadoc/com/hazelcast/core/MultiMap.html

ps: When removing a value from a  MultiMap, Hazelcast iterates through value collection and checks equality one by one (deserializing one-by-one). 

@mmdogan

[Tim Peierls]

This is all fine, and the javadocs are good, but I do think it's worth explicitly encouraging people to continue to supply equals and hashCode on all types used in key-like settings, so that they can be used safely outside of Hazelcast.

--tim

[Jason Clawson]

I think you guys might have to really point out the exact sentence in the docs that says this.  I am just not seeing it :-(.

http://hazelcast.com/docs/2.1/javadoc/com/hazelcast/core/MultiMap.html#remove(java.lang.Object,%20java.lang.Object)

It doesn't say hashCode and equals are required there for the value.  Plus, if this is how it is intended to work, why does containsValue compare the serialized form of the value?  There is a bug here.  Maybe it's a documentation + consistency thing.

On another note, I would not expect the implementation of remove(k,v) to iterate over every single value in k.  I would expect that to be optimized in hazelcast to be O(1) and I don't think this is an unreasonable assumption for this to be the implementation.  For example, it would send a message to the owner of k to remove v.  The owner of k would then complete this in O(1) using HashSet.remove(v).  (this was my assumption)

I agree that keys and values should always implement consistent equals and hashCodes.  I would just like to see that documented and the API consistent with whatever mechanism you choose.  

I had assumed that Hazelcast was trying to be as fast as possible, opting to store data (keys and values) in its serialized form so it's fast to send keys/values over the wire without having to serialize all the time.  I assumed that any comparison request that came over the wire would then compare the serialized forms so it did not have to incur the cost of n deserializations.  This makes the most sense to me from a performance perspective.

tl;dr ... I honestly don't think hazelcast should ever use equals or hashCode for comparing keys or values.  Not using them makes the API consistent.  It allows you to do a lot of optimizations.

Thanks,

Jason

Sent from my iPhone

[Jason Clawson]

Here is another strange result.  It seems multimap.remove(k,v) functions a little unpredictably when it is in a transaction.  This seems to be due to the fact that it uses a mix of comparing the serialized equality with the equals() method.  So, if they are not consistent for whatever reason, you will get strange side effects.

MultiMap<Integer, Order> mmap = Hazelcast.getMultiMap("test");

mmap.put(1, new Order(1));

Hazelcast.getTransaction().begin();

System.out.println(mmap.remove(1, new Order(1)));

Hazelcast.getTransaction().commit();

System.out.println(mmap.size());

When order doesn't have equals and hashCode implemented the program above outputs:

true

1

It is strange that it outputs "true".  This is because when you are in a transaction it compares the serialized identity of the value.

[Jason Clawson]

Or how about this example:

MultiMap mmap = Hazelcast.getMultiMap("test");

mmap.put(1, 2);

Hazelcast.getTransaction().begin();

System.out.println(mmap.remove(1, 2L));

Hazelcast.getTransaction().commit();

System.out.println(mmap.size());

This outputs:

true

1

Whereas it should return:

false

1

Because ((Long)2).equals((Integer)2) == false but the serialized forms are equal.  So, if any Serialized form == another Serialized form regardless of type you are going to have a bad time if you are using transactions because of the inconsistency.

[Tim Peierls]

I am not part of the Hazelcast team, so I can't respond to much of this.

My point is that until and unless the Hazelcast team changes the rules, the safe way to work with Hazelcast right now is to define equals and hashCode methods for distributed objects to be consistent with the serialized forms of those objects.

Since that's not always possible or desirable when using default Java serialization, I think it's best to handle serialization oneself, which is currently achievable only by implementing DataSerializable. (That might change soon, though.) 

--tim

--

You received this message because you are subscribed to the Google Groups "Hazelcast" group.

To view this discussion on the web visit https://groups.google.com/d/msg/hazelcast/-/WceKIXn61sAJ.

To post to this group, send email to haze...@googlegroups.com.

To unsubscribe from this group, send email to hazelcast+...@googlegroups.com.

[Jason Clawson]

I agree with you.  You should always implement consistent hashCode and equals when putting things in collections.  My real point is here... it doesn't matter.  Due to the inconsistency in how its handled in hazelcast you could implement hashCode and equals all you want and see strange side effects.  Lets say you have 2 classes:  Foo, and Bar and both implement DataSerializable and just have a single integer inside and you implements hashCode and equals correctly (eclipse-- generate hashCode and equals).

Then do:

MultiMap mmap = Hazelcast.getMultiMap("foo");

mmap.put("1", new Bar(1));

Hazelcast.getTransaction().begin();

System.out.println(mmap.remove("1", new Foo(1)));

Hazelcast.getTransaction().commit();

System.out.println(mmap.size());

That will output:

true

1

That's my point.

[Tim Peierls]

It's possible that we are in violent agreement, but just to be clear: I wasn't referring to the need for consistency between equals and hashCode -- equal objects must have equal hash codes or regular maps don't work -- I was saying that for types T that appear as type parameters of Hazelcast data structures, it is a requirement that for any a and b of type T, a.equals(b) iff the serialized forms of a and b are byte-for-byte identical.

In the examples you give, this requirement is not met: The type of the object added is different from the type of object used to remove (Long vs. Integer, Bar vs. Foo), and it is not the case that a.equals(b) iff a and b are byte-for-byte identical in their serialized forms.

I agree that the documentation doesn't go far enough in spelling out this requirement for users.  

--tim

[Jason Clawson]

We do agree on equals and hashCode :-).  I think the requirement of consistency between those 2 methods have been documented pretty well by java.  

However, I think you missed the point on my examples.  And the requirement you speak of IS met.  I don't see how you can say it is not.  Take my example posted earlier and add generic params:  MultiMap<Integer, Number> ... It will work the same and the generics contract is fulfilled.

I highly encourage you to try it out in code so you can see what I am talking about.

My point is that MultiMap functions differently depending on if remove(k,v) is called within a transaction or not.  If its called within a transaction it compares the identity of v in its serialized form.  In my previous example the call to remove(k,v) should return false because it is NOT equal.  It just happens that its serialized byte form IS equal which is why remove(k,v) returns true when its done within a transaction --- (note: when the transaction is committed it doesn't actually remove the element even though it previously said it would).  

If the Hazelcast devs want value comparisons to be done with equals & hashCode... that's fine... but it needs to be consistent and I think I have shown that it is not.  I would still maintain that you could achieve higher performance if you did not use equals & hashCode to do the value comparisons.

Thanks,

Jason

[Jason Clawson]

I will say that for the original issue I was having, adding equals and hashCode fixed it, so thank you for pointing that out :-).  I just delved deeper into how MultiMap works and found this new issue and it bugged me.

Thanks

To unsubscribe from this group, send email to hazelcast+unsubscribe@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/hazelcast?hl=en.

--
You received this message because you are subscribed to the Google Groups "Hazelcast" group.
To post to this group, send email to haze...@googlegroups.com.

To unsubscribe from this group, send email to hazelcast+unsubscribe@googlegroups.com.

For more options, visit this group at http://groups.google.com/group/hazelcast?hl=en.

--
You received this message because you are subscribed to the Google Groups "Hazelcast" group.
To post to this group, send email to haze...@googlegroups.com.

To unsubscribe from this group, send email to hazelcast+unsubscribe@googlegroups.com.

[Tim Peierls]

On Thu, Jun 28, 2012 at 5:48 PM, Jason Clawson <jcla...@qualys.com> wrote:

However, I think you missed the point on my examples.  And the requirement you speak of IS met.  I don't see how you can say it is not.  Take my example posted earlier and add generic params:  MultiMap<Integer, Number> ... It will work the same and the generics contract is fulfilled.

I don't see it. The requirement is a.equals(b) iff a and b have identical serialized forms. (Note: iff, not if.)  But you say this:

 

In my previous example the call to remove(k,v) should return false because it is NOT equal.  It just happens that its serialized byte form IS equal which is why remove(k,v) returns true when its done within a transaction --- (note: when the transaction is committed it doesn't actually remove the element even though it previously said it would).  

So you have two things that are !equals but with the same serialized form, which violates the requirement.

 

My point is that MultiMap functions differently depending on if remove(k,v) is called within a transaction or not.  If its called within a transaction it compares the identity of v in its serialized form.  

If the Hazelcast devs want value comparisons to be done with equals & hashCode... that's fine... but it needs to be consistent and I think I have shown that it is not.  I would still maintain that you could achieve higher performance if you did not use equals & hashCode to do the value comparisons.

That's a different issue, one that has to wait for the Hazelcast team to respond to.

--tim

[Jason Clawson]

The serialized form of Long appears to be equal to the serialized form of Integer for the same number for the purposes of MultiMap.remove(k,v) within a transaction. (2 vs 2L)  Please run my example code.  I am not trying to have a philosophical discussion on serialization.  I just want to point out a bug I found.  If you think its not a bug, please feel free to post some code and I will happily run it.

Thanks,

Jason

--

[Tim Peierls]

On Thu, Jun 28, 2012 at 6:21 PM, Jason Clawson <jcla...@qualys.com> wrote:

The serialized form of Long appears to be equal to the serialized form of Integer for the same number for the purposes of MultiMap.remove(k,v) within a transaction. (2 vs 2L)  Please run my example code.  I am not trying to have a philosophical discussion on serialization.  I just want to point out a bug I found.  If you think its not a bug, please feel free to post some code and I will happily run it.

I believe that your code produces the output you say it does; do I really need to run it to confirm that?

I understand what you think correct output should be, but I'm saying all bets are off with Hazelcast if you don't have equals consistent with serialized form. That's what those gotcha clauses imply -- it's not a philosophical point! -- I just wish that the javadocs were more explicit about the implications of those gotchas.

So while there might be a bug, your code doesn't demonstrate it. All it says is that you have to be careful what types you pass to get/contains/remove/etc. for Hazelcast data structures. Trying to remove a Long by passing an Integer is not guaranteed to behave consistently.

I'd still like to hear from Hazelcast folks about whether any of the restrictions could be lifted in the future. Even with the full implications of the gotchas expressed in Javadocs, people are going to have their expectations dashed.

--tim

[Jason Clawson]

Can you post the code you would implement for Foo and Bar implementing DataSerializable such that it illustrates your point (both containing a single int)?

--

[Tim Peierls]

I don't think there are any values for Foo and Bar that make sense. I don't accept the form of your example: Attempts to remove something using a different type aren't guaranteed to work.

[Jason Clawson]

This is that last time I am going to try and explain the bug... and even simpler this time.  Again, the functionality of remove(k,v) is inconsistent depending if a transaction is open.  Let me make another example... and this is a crazy simple one... no serialization involved in this one...

MultiMap<Integer, String> mmap = Hazelcast.getMultiMap("mmap");

mmap.put(1, "Foo");

System.out.println(mmap.remove(1, "Bar"));

Hazelcast.getTransaction().begin();

System.out.println(mmap.remove(1, "Bar"));

Hazelcast.getTransaction().commit();

This outputs:

false

true

Yup... in a transaction it doesn't matter WHAT you put for v!!!  It always reports it removed something for the first call.

[Tim Peierls]

OK! That looks like a bug. 

--

[Mehmet Dogan]

On Thu, Jun 28, 2012 at 6:05 PM, Jason Clawson <jcla...@qualys.com> wrote:

I think you guys might have to really point out the exact sentence in the docs that says this.  I am just not seeing it :-(.

http://hazelcast.com/docs/2.1/javadoc/com/hazelcast/core/MultiMap.html#remove(java.lang.Object,%20java.lang.Object)

It doesn't say hashCode and equals are required there for the value.  Plus, if this is how it is intended to work, why does containsValue compare the serialized form of the value?  There is a bug here.  Maybe it's a documentation + consistency thing.

Yes, it doesn't say hashCode and equals are required there for the value. But it says key's hashCode and equals will not used here, eventually it means for value they are required. Sure, doc should be more explicit about these things. 

containsValue also uses hashCode and equals, it seems that is an Transaction issue.

 

On another note, I would not expect the implementation of remove(k,v) to iterate over every single value in k.  I would expect that to be optimized in hazelcast to be O(1) and I don't think this is an unreasonable assumption for this to be the implementation.  For example, it would send a message to the owner of k to remove v.  The owner of k would then complete this in O(1) using HashSet.remove(v).  (this was my assumption)

Yes, caller sends owner of k to remove v. What I said about iteration is for the worst case, which happens when value type of MultiMap is a List not a Set. If value type is Set then call will end up at HashSet.remove(v).

http://hazelcast.com/docs/2.1/javadoc/com/hazelcast/config/MultiMapConfig.html#setValueCollectionType(com.hazelcast.config.MultiMapConfig.ValueCollectionType)

  

I had assumed that Hazelcast was trying to be as fast as possible, opting to store data (keys and values) in its serialized form so it's fast to send keys/values over the wire without having to serialize all the time.  I assumed that any comparison request that came over the wire would then compare the serialized forms so it did not have to incur the cost of n deserializations.  This makes the most sense to me from a performance perspective.

You are the first one saying Hazelcast should use binary form instead of original equals and hashCode when comparing values. :)

In previous versions Hazelcast was using serialized form always, but people wanted us to change this behavior to use equals and hashCode that they implemented.

 

 

[Mehmet Dogan]

So bug here is under transaction MultiMap reports it removed something that it should not, but actually it does not remove anything.

@mmdogan

[Jim Johnson]

I'm having similar trouble in my upgrade from 2.1 to 3.2.  My multimap contains a String->POJO mapping where POJO has a specific hashcode & equals defined.  In 2.1 calling multimap.remove(key, pojo) my object would be removed so I can replace it with a subsequent put call.  In 3.2 the pojo that my multimap has does not match against the one I'm passing in anymore.  In the end I end up with 2 objects in the multimap, both of which the equals() & hashCode() match each other.

Is this expected?  If so what is the recommended way to interact with multimap?  Thanks!

-Jim

[giri.ve...@gmail.com]

Hello, I am having a similar issue. Any resolution for your reported issue?

It seems like if the POJO contains any collection data, for sure it fails to remove - otherwise it seem to remove. 

Do we have a good way to consistently affect remove on a multimap?

Thanks,

Giri

[Ali Gurbuz]

Hi,

I've amended the test code (added a collection in it) but couldn't reproduce the issue

Which version are you using ? I've tried with the latest development branch(3.5-SNAPSHOT)

public class MultimapBug {

    public static void main(String[] args) {

        Config config = new Config();
        config.getNetworkConfig().getJoin().getMulticastConfig().setEnabled(false);
        HazelcastInstance instance = Hazelcast.newHazelcastInstance(config);
        MultiMap<Integer, Order> multiMap = instance.getMultiMap("blah");
        multiMap.put(1, new Order(1, "foo"));
        multiMap.put(1, new Order(2, "bar"));
        multiMap.put(1, new Order(3, "zoo"));

        multiMap.put(2, new Order(4, "foo"));
        multiMap.put(2, new Order(5, "bar"));
        multiMap.put(2, new Order(6, "zoo"));

        Collection<Order> items = multiMap.get(1);
        for (Order o : items) {
            System.out.println(multiMap.remove(1, o));
        }

        System.out.println("Size: " + multiMap.size());

    }

    public static class Order implements Serializable {
        private static final long serialVersionUID = 1L;
        private int id;

        private Set<String> set;

        public Order(int id, String s) {
            this.id = id;
            set = new HashSet<String>();
            set.add(s);
        }
    }
}

--
You received this message because you are subscribed to the Google Groups "Hazelcast" group.

To unsubscribe from this group and stop receiving emails from it, send an email to hazelcast+...@googlegroups.com.

To post to this group, send email to haze...@googlegroups.com.

Visit this group at http://groups.google.com/group/hazelcast.
To view this discussion on the web visit https://groups.google.com/d/msgid/hazelcast/b66df3f6-c695-4648-83a2-48bd3e4eed3e%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Ali Gurbuz
Solutions Architect
Mahir İz Cad. No:35, Altunizade, İstanbul
a...@hazelcast.com 
+90 507 857 7815
@aligurbuz