[jira] [Work logged] (ARTEMIS-2007) Messages not redistributed to consumers with matching filters when local non matching consumers are present

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/ARTEMIS-2007?focusedWorklogId=632901&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-632901 ]

ASF GitHub Bot logged work on ARTEMIS-2007:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 03/Aug/21 12:59
            Start Date: 03/Aug/21 12:59
    Worklog Time Spent: 10m 
      Work Description: gemmellr commented on a change in pull request #3635:
URL: https://github.com/apache/activemq-artemis/pull/3635#discussion_r681731532



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -1567,7 +1566,14 @@ protected void finalize() throws Throwable {
 
    @Override
    public int getConsumerCount() {
-      return consumers.size();
+      // we don't want to count the redistributor, it is an internal transient entry in the consumer list
+      // check for presence before checking consumers to align with use of set()
+      final boolean compensateForPresenceOfRedistributor = redistributorUpdater.get(this) != null;
+      int size = consumers.size();
+      if (size > 0 && compensateForPresenceOfRedistributor) {
+         size--;
+      }
+      return size;

Review comment:
       Sine this isnt done under lock and both the redistributor and consumers list could be changing independently, it seems quite racey? Seems to be used in some important places so I'd wonder if that is a problem.
   
   Given that 'consumers' is actually a custom structure, and updates to its underlying structure are done under lock, it coudl perhaps be made to handle this internally (and e.g add a specific compensating size method)?

##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java
##########
@@ -140,6 +139,7 @@
    private static final AtomicLongFieldUpdater<QueueImpl> dispatchStartTimeUpdater = AtomicLongFieldUpdater.newUpdater(QueueImpl.class, "dispatchStartTime");
    private static final AtomicLongFieldUpdater<QueueImpl> consumerRemovedTimestampUpdater = AtomicLongFieldUpdater.newUpdater(QueueImpl.class, "consumerRemovedTimestamp");
    private static final AtomicReferenceFieldUpdater<QueueImpl, Filter> filterUpdater = AtomicReferenceFieldUpdater.newUpdater(QueueImpl.class, Filter.class, "filter");
+   private static final AtomicReferenceFieldUpdater<QueueImpl, ConsumerHolder> redistributorUpdater = AtomicReferenceFieldUpdater.newUpdater(QueueImpl.class, ConsumerHolder.class, "redistributor");

Review comment:
       Is this needed? Seems like there are no CAS etc operations done on it, just straight sets, and all actual uses of the redistributor field except one are done under synchronized methods/blocks, except the use in getConsumerCount where it just does a get with this updater vs using the volatile-anyway field. (The subsequent usage there is perhaps not so safe as it isnt under lock)




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Issue Time Tracking
-------------------

    Worklog Id:     (was: 632901)
    Time Spent: 4h 40m  (was: 4.5h)

> Messages not redistributed to consumers with matching filters when local non matching consumers are present
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: ARTEMIS-2007
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-2007
>             Project: ActiveMQ Artemis
>          Issue Type: Improvement
>          Components: AMQP, Broker
>    Affects Versions: 2.6.3
>            Reporter: Sebastian T
>            Assignee: Gary Tully
>            Priority: Major
>         Attachments: AMQ2007Test.java, artemis-2007.zip
>
>          Time Spent: 4h 40m
>  Remaining Estimate: 0h
>
> We are experiencing the following issue:
> # We configure an Artemis cluster with ON_DEMAND message loadbalacing and message redistribution enabled.
> # We then connect a single consumer to *queues.queue1* on node1 that has a message filter that does NOT match a given message.
> # Then we send a message to *queues.queue1* on node1.
> # Then we connect a consumer to *queues.queue1* on node2 that has a filter matching the message we sent.
> We now would expect that the message on node1 currently not having any matching consumers on node1 to be forwarded or redistributed to node2 where a matching consumer exists.
> However that is not happening the consumer on node2 does not receive the message and in our case the message on node1 expires after some time despite a matching consumer is connected to the cluster.
> In the described scenario when we disconnect the consumer on node1 (that does not match the message anyway) the message is redistributed to node2 and consumed by the matching consumer.
> If no consumer was connected to node1, a message is sent to node1 and only then a matching consumer is connected to node2 the message is forwarded to node2 as expected.
> So I guess the core problem is that message redistribution of messages on node1 is not triggered when a matching consumer is connected to node2 while a *any* consumer already exists on node1 no matter if it actually matches the given message.
> I attached a maven test case that illustrates the issue.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)