[jira] [Commented] (AMQ-5322) Message is assigned to different client with the same JMSXGroupID

["Luis Rojas (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/AMQ-5322?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14123478#comment-14123478 ] 

Luis Rojas commented on AMQ-5322:
---------------------------------

I came across the code looking for a solution to our problems in production. We open many JMSXGroupIds simultaneously, and messages that should arrive to only one consumer arrive to any other consumer, like there was no previous association for that GroupId.

I saw the implementation of MessageGroupHashBucket and it is just plain wrong. That is a very bad implemented Hash.
It does not consider that independent of the value of bucketCount, the hashCode() can be the same for two different groupIds.

If we see the code here :

http://activemq.apache.org/maven/5.10.0/apidocs/src-html/org/apache/activemq/broker/region/group/MessageGroupHashBucket.html

134    protected int getBucketNumber(String groupId) {
135        int bucket = groupId.hashCode() % bucketCount;
136        // bucket could be negative
137        if (bucket < 0) {
138            bucket *= -1;
139        }
140        return bucket;
141    }
 
For two different groupIds, the value returned by getBucketNumber() can be the same. That is called a "collision" in a hash.
Now, what happens if we want to put a new GroupId associated with a different ConsumerId into an already used bucket  ?

041    public synchronized void put(String groupId, ConsumerId consumerId) {
042        int bucket = getBucketNumber(groupId);
043        consumers[bucket] = consumerId;
044        if (consumerId != null){
045          cache.put(groupId,consumerId.toString());
046        }
047    }

OMG!!! There is no validation at all. The previous value is simply overwriten !!
We use UUID as GroupIds, and no matter that they are all different, many times the getBucketNumber() is the same, the value overwritten and the previous association is lost.
It is not a solution to increase the value of bucketCount, it is not a matter of size of the Hash, it is really a bad implementation.

More about collisions here :

https://www.princeton.edu/~achaney/tmve/wiki100k/docs/Hash_collision.html

A hash should deal with collisions, it is part of problems of a hash, and there are different solutions. For instance, each bucket could be a list of associated GroupIds. Unfortunately then a lineal search is needed and performance is affected. That is why the Hashes are usually so dependant of the hashCode() implementation.


> Message is assigned to different client with the same JMSXGroupID
> -----------------------------------------------------------------
>
>                 Key: AMQ-5322
>                 URL: https://issues.apache.org/jira/browse/AMQ-5322
>             Project: ActiveMQ
>          Issue Type: Bug
>          Components: Broker
>    Affects Versions: 5.9.0, 5.9.1, 5.10.0
>            Reporter: wesley lin
>         Attachments: AMQ5322Test.java, GroupIdTest.java
>
>
> I discovered since AMQ 5.9, message could be sent to different consumer even with JMSXGroupID is set.
> Here is how to reproduce.
> 1. start 10 consumers to connect to server (consumer 1-10)
> 2. send 1025 messages with JMSXGroupID set from 0 - 1024 
> 3. add other 10 consumers to connect to server (consumer 11-20)
> 4. send several messages with JMSXGroup set from 0 - 1024
> Now, you can observe consumers added lately ( 11 - 20 ) will also receive messages even JMSXGroupID have been dispatched to first consumer group( 1 - 10).
>  
> note:  In step2 , there should have more than 1024 different kind of JMSXGroupID to reproduce this bug.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)