Questio on setting group association in delivery algo

[timfox]

Another question:

Currently the group--consumer association is set before the consumer has handled the reference:

final SimpleString groupID = reference.getMessage().getSimpleStringProperty(MessageImpl.HDR_GROUP_ID);

 if (groupID != null)
 {
 Consumer groupConsumer = groups.putIfAbsent(groupID, consumer);

 if (groupConsumer != null && groupConsumer != consumer)
 {
 continue;
 }
 }

 HandleStatus status = handle(reference, consumer);

And then, if the consumer doesn't handle it, it is removed:

else if (status == HandleStatus.NO_MATCH)
 {
 // if consumer filter reject the message make sure it won't be assigned the message group
 if (groupID != null)
 {
 groups.remove(consumer);
 }
 }

Why not just set it after the consumer has handled it, then you know it's been handled?

[jmesnil]

yes, it'd be simpler to set the consumer after the message is handled.

besides, the current code is dead wrong:

else if (status == HandleStatus.NO_MATCH)
 {
 // if consumer filter reject the message make sure it won't be assigned the messag
e group
 if (groupID != null)
 {
 groups.remove(consumer);
 }
 }

groups.remove() must be called with the key (groupID), not with the value (consumer).
As it is written now, it is possible to keep a consumer with a non-matching filter to a group!

[timfox]

Good catch.

Another thing I don't understand is this:

if (groupConsumer != null && groupConsumer != consumer)
 {
 continue;
 }

I don't see how this code would ever be executed since the group association is always protected by a synchronized lock.

[timfox]

Actually, scratch that. The code is done before handle for a purpose, otherwise the wrong consumer could get the message.