You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@qpid.apache.org by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org> on 2007/09/04 20:14:45 UTC

[jira] Created: (QPID-572) broker delivers messages out of order

broker delivers messages out of order
-------------------------------------

                 Key: QPID-572
                 URL: https://issues.apache.org/jira/browse/QPID-572
             Project: Qpid
          Issue Type: Bug
          Components: Java Broker
            Reporter: Rafael H. Schloming


ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.

I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.

I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Re: [jira] Commented: (QPID-572) broker delivers messages out of order

Posted by Rafael Schloming <ra...@redhat.com>.
Martin Ritchie wrote:
> IMO the modified fix is the correct fix. As the TopicSessionTest will
> still fail with the first fix.
> 
> The logic is as follows:
> 
> Summarising the Topic Session Test:
> 
> A subscribe (Sn) is created that receives all messages on the topic
> A subscriber (Ss) is created with a selector.
> 
> A message (m1) is sent to the topic which is received by Sn
> The CSDM logic for Ss is as follows:
> A call to nextSuscriber(m1) is made to locate any subscribers on this
> queue (which given we are talking "topics" is exclusive queues with
> only the give S attached.
> 
> However, in this case Ss has a selector that doesn't match m1. So
> nextSubscrier(m1) will return null. As there are no subscribers on
> this queue that are willing and able to receive the message.
> 
> The two fixes for CSDM both are true here:
> First Fix:
> if (s == null || hasQueuedMessages() )
> Modified Fix:
> if (s == null || (!s.filtersMessages() && hasQueuedMessages() ))
> 
> So s is null so queuing is performed, This actually is a bug
> (QPID-545) as this message will NEVER be consumed as it doesn't match
> Ss' filter. However it will be added to the mail queue.
> 
> On receipt of a second message (m2) that does match the filter, the
> first fix will fail as s is now non null and hasQueuedMessages()
> returns true. Causing queuing BUT there is no queuing on Ss. So TST
> will fail as it expects to receive m2. The Modified fix prevents
> checking the main Queue when s is a filtering subscriber as
> nextSubscriber has already checked the PDG for content.
> 
> QPID-545 would prevent the need for the modified fix. However, I have
> not had time to write a test to verify my solution for QPID-545.

I believe a similar scenario can happen based solely on timing, without 
the first message entering the picture. See QPID-601 for details.

On a slight tangent, QPID-545 is a bit of an odd beast. It's not 100% 
apparent to me that it is actually a bug. There seems to be a 
presumption that there can be only one subscriber to a private queue, 
however I don't think this is actually the case. A private queue is 
exclusive to the connection, which means there could be multiple 
sessions from the same connection subscribing to a single queue.

It may be that JMS semantics constrain things sufficiently that the 
presumption is true, however if this is the case then there may need to 
be some reconciliation between JMS and AMQP semantics.

--Rafael


Re: [jira] Commented: (QPID-572) broker delivers messages out of order

Posted by Martin Ritchie <ri...@apache.org>.
IMO the modified fix is the correct fix. As the TopicSessionTest will
still fail with the first fix.

The logic is as follows:

Summarising the Topic Session Test:

A subscribe (Sn) is created that receives all messages on the topic
A subscriber (Ss) is created with a selector.

A message (m1) is sent to the topic which is received by Sn
The CSDM logic for Ss is as follows:
A call to nextSuscriber(m1) is made to locate any subscribers on this
queue (which given we are talking "topics" is exclusive queues with
only the give S attached.

However, in this case Ss has a selector that doesn't match m1. So
nextSubscrier(m1) will return null. As there are no subscribers on
this queue that are willing and able to receive the message.

The two fixes for CSDM both are true here:
First Fix:
if (s == null || hasQueuedMessages() )
Modified Fix:
if (s == null || (!s.filtersMessages() && hasQueuedMessages() ))

So s is null so queuing is performed, This actually is a bug
(QPID-545) as this message will NEVER be consumed as it doesn't match
Ss' filter. However it will be added to the mail queue.

On receipt of a second message (m2) that does match the filter, the
first fix will fail as s is now non null and hasQueuedMessages()
returns true. Causing queuing BUT there is no queuing on Ss. So TST
will fail as it expects to receive m2. The Modified fix prevents
checking the main Queue when s is a filtering subscriber as
nextSubscriber has already checked the PDG for content.

QPID-545 would prevent the need for the modified fix. However, I have
not had time to write a test to verify my solution for QPID-545.

On 14/09/2007, Rafael H. Schloming (JIRA) <qp...@incubator.apache.org> wrote:
>
>     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527624 ]
>
> Rafael H. Schloming commented on QPID-572:
> ------------------------------------------
>
> I believe the original fix for this issue is actually more correct than the modified fix, although the modified fix might be considered a reasonable M2 workaround for QPID-601. Please see my comments on QPID-601 for details.
>
> > broker delivers messages out of order
> > -------------------------------------
> >
> >                 Key: QPID-572
> >                 URL: https://issues.apache.org/jira/browse/QPID-572
> >             Project: Qpid
> >          Issue Type: Bug
> >          Components: Java Broker
> >    Affects Versions: M2, M2.1, M3
> >            Reporter: Rafael H. Schloming
> >         Attachments: QPID-572-testcase.patch
> >
> >
> > ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> > I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> > I have only verfied this issue on the trunk, however I believe it effects M2 as well.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>


-- 
Martin Ritchie

Re: [jira] Commented: (QPID-572) broker delivers messages out of order

Posted by Martin Ritchie <ri...@apache.org>.
The patch i provided doesn't solve the ordering. But i have tested a
real fix. This problem is also causing the topic session test to fail.
Will fix all branches tomorrow.

On 13/09/2007, Rafael H. Schloming (JIRA) <qp...@incubator.apache.org> wrote:
>
>     [
> https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527194
> ]
>
> Rafael H. Schloming commented on QPID-572:
> ------------------------------------------
>
> I'm now seeing the same failur on the M2 branch as I saw on the trunk when I
> tried the same fix originally. In particular I am now reliably seeing the
> following failure in TopicSessionTest:
>
> junit.framework.AssertionFailedError
>         at junit.framework.Assert.fail(Assert.java:47)
>         at junit.framework.Assert.assertTrue(Assert.java:20)
>         at junit.framework.Assert.assertNotNull(Assert.java:220)
>         at junit.framework.Assert.assertNotNull(Assert.java:213)
>         at
> org.apache.qpid.test.unit.topic.TopicSessionTest.testNoLocal(TopicSessionTest.java:338)
>
> It occurs reliably when I run the full suite of client tests, however it
> does not occur when I run TopicSessionTest on its own. I believe this is
> because selectors do not function properly when there are messages queued
> for asynchronous delivery, and this change makes that situation more likely.
>
> > broker delivers messages out of order
> > -------------------------------------
> >
> >                 Key: QPID-572
> >                 URL: https://issues.apache.org/jira/browse/QPID-572
> >             Project: Qpid
> >          Issue Type: Bug
> >          Components: Java Broker
> >    Affects Versions: M2, M2.1, M3
> >            Reporter: Rafael H. Schloming
> >         Attachments: QPID-572-testcase.patch
> >
> >
> > ConcurrentSelectorDeliveryManager will sometimes deliver messages out of
> order. This is caused by the code in deliver(...) that attempts to
> short-circuit message queuing when there is an available subscription. This
> code can result in the currently published message skipping ahead of queued
> messages causing out of order delivery. Although unrelated to transactions,
> I have observed this failure occuring in TransactedTest both in testCommit
> and testRollback. Normally it does not happen very frequently, however
> placing a Thread.sleep(500) in the async delivery thread will cause the
> failure to occur almost all the time.
> > I tried fixing the problem by only attempting synchronous delivery when
> there are no queued messages, however this appears to break other tests that
> use selectors. This makes me suspect that the selector implementation is
> somehow incorrectly coupled to synchronous delivery.
> > I have only verfied this issue on the trunk, however I believe it effects
> M2 as well.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>


-- 
Martin Ritchie

[jira] Assigned: (QPID-572) broker delivers messages out of order

Posted by "Aidan Skinner (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aidan Skinner reassigned QPID-572:
----------------------------------

    Assignee: Rafael H. Schloming  (was: Aidan Skinner)

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>            Assignee: Rafael H. Schloming
>             Fix For: M3
>
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526148 ] 

Rafael H. Schloming commented on QPID-572:
------------------------------------------

That's what I did. With those steps it occurs consistently for me when running TransactedTest. Were you testing on the trunk or M2? If M2 then you could try the same steps on the trunk where the bug is known to exist. If it doesn't occur for you on the trunk then we know that it is a timing issue related to our different environments.

It's possible that your extra cores are making the problem harder to reproduce since your consumer will be running on its own core. Reproducing the problem requires there to be messages backed up in the queue in order for the async delivery thread to be able to kick in. If your consuming thread has its own core, that may never happen. It may be that we need to rate limit the consumer in order to get the same effect on your machine. You could try putting an additional Thread.sleep() in dispatchMessage inside AMQSession.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (QPID-572) broker delivers messages out of order

Posted by "Rob Godfrey (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rob Godfrey resolved QPID-572.
------------------------------

    Resolution: Fixed
      Assignee: Rob Godfrey  (was: Rafael H. Schloming)

Fixed by the refactoring which removed CSDM

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>            Assignee: Rob Godfrey
>             Fix For: M3
>
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Martin Ritchie updated QPID-572:
--------------------------------

    Attachment: CSDM.patch

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: CSDM.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527194 ] 

Rafael H. Schloming commented on QPID-572:
------------------------------------------

I'm now seeing the same failur on the M2 branch as I saw on the trunk when I tried the same fix originally. In particular I am now reliably seeing the following failure in TopicSessionTest:

junit.framework.AssertionFailedError
        at junit.framework.Assert.fail(Assert.java:47)
        at junit.framework.Assert.assertTrue(Assert.java:20)
        at junit.framework.Assert.assertNotNull(Assert.java:220)
        at junit.framework.Assert.assertNotNull(Assert.java:213)
        at org.apache.qpid.test.unit.topic.TopicSessionTest.testNoLocal(TopicSessionTest.java:338)

It occurs reliably when I run the full suite of client tests, however it does not occur when I run TopicSessionTest on its own. I believe this is because selectors do not function properly when there are messages queued for asynchronous delivery, and this change makes that situation more likely.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Robert Greig (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526031 ] 

Robert Greig commented on QPID-572:
-----------------------------------

Rafi, I have tried reproducing this. Hardware is a 4 way dual-core opteron box running RHEL 4. Java 6 update 2.

I ran the tests several times without modifying the code and they all passed.

I tried adding in a sleep (500) and the tests still passed. Maybe I didn't add the sleep to the right place? I added it to the ConcurrentSelectorDeliveryManager, to run run() method of the inner Runner class:

       public void run()
        {
            boolean running = true;
            while (running && !_movingMessages.get())
            {
                  try { Thread.sleep(500); } catch (Exception e) {}
                processQueue();

Is that right? I haven't had much time to investigate the problem so I was really just trying to make the changes and run the tests a few times to see what happens.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12525726 ] 

Martin Ritchie commented on QPID-572:
-------------------------------------

the nextSubscriber method used by the deliver(...) method MUST only return a subscriber that is ready and able to receive a message.
i.e. it is not suspended and there are no queued messages for that subscriber.

The bug is that the SubscriptionImp only checks if it is queuing msgs when there is a filter in place. If there is no filter it doesn't check the main queue for messages. 

            if (!(subscription.isSuspended() || subscription.wouldSuspend(msg)))
            {
                if (subscription.hasInterest(msg))
                {
                    // if the queue is empty then this client is ready to receive a message.
                    //FIXME the queue could be full of sent messages.
                    // Either need to clean all PDQs after sending a message
                    // OR have a clean up thread that runs the PDQs expunging the messages.
>>>>>        if (!subscription.filtersMessages() || subscription.getPreDeliveryQueue().isEmpty())

// Should be something like this:
/// if there is a filter does this subscription have any pending messages || if there are no messages pending(This method doesn't exist).
                    if ((subscription.filtersMessages() && subscription.getPreDeliveryQueue().isEmpty() ) ||  subscription.getDeliveryQueue().isEmpty )



> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Martin Ritchie updated QPID-572:
--------------------------------

    Attachment:     (was: CSDM.patch)

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Robert Greig (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526151 ] 

Robert Greig commented on QPID-572:
-----------------------------------

Ah of course I had forgotten that the async thread may never be invoked at all.

I will put in the additional sleep then test against trunk if that still shows up "clean".


> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12579380#action_12579380 ] 

Martin Ritchie commented on QPID-572:
-------------------------------------

Rafi,

Do you belive that the changes made has addressed this issue? 

Cheers

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Rafael H. Schloming updated QPID-572:
-------------------------------------

    Affects Version/s: M3
                       M2.1
                       M2

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Robert Greig (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526152 ] 

Robert Greig commented on QPID-572:
-----------------------------------

OK with a sleep in the dispatchMessage method it fails on M2 too.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (QPID-572) broker delivers messages out of order

Posted by "Aidan Skinner (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aidan Skinner updated QPID-572:
-------------------------------

    Attachment: QPID-572-testcase.patch

patch to add a test case to cover this situation

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: CSDM.patch, QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526191 ] 

Rafael H. Schloming commented on QPID-572:
------------------------------------------

Hmm, interesting...

That is almost the exact same as the fix I tried, however it seemed to cause quite a few failures when I tried it. In particular selectors no longer seemed to reliably work. I'm in the middle of a few changes currently, however when my checkout is clean I'll try applying your patch and seeing if it works for me. My version of the fix was slightly different in that it didn't call nextSubscriber() at all if hasQueuedMessages() returned true, otherwise it was the same.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: CSDM.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rob Godfrey (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12621765#action_12621765 ] 

Rob Godfrey commented on QPID-572:
----------------------------------

As described in QPID-950, the subscriber now uses a pointer to the strictly ordered queue and will only attempt delivery from the queue - it cannot be "short-circuited".  On message arrival a fast-path is used if it is detected that the message would be the next one that a subscription would consider delivering.  See the description of QPID-950 for an overview of the design of the new queuing model.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>            Assignee: Rob Godfrey
>             Fix For: M3
>
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Updated: (QPID-572) broker delivers messages out of order

Posted by "Aidan Skinner (JIRA)" <qp...@incubator.apache.org>.
     [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Aidan Skinner updated QPID-572:
-------------------------------

    Fix Version/s: M3

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>             Fix For: M3
>
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12525760 ] 

Rafael H. Schloming commented on QPID-572:
------------------------------------------

As far as I can tell there is no Subscription.getDeliveryQueue() method.

Also, I'm unsure how message selectors are supposed to work for the async delivery path, and as I said above I'm a bit concerned that turning of the synchronous delivery path caused selectors to break. Isn't it true that the synchronous delivery path is just an optimization? Where exactly is the message filtering performed for the asynchronous delivery path?

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526188 ] 

Martin Ritchie commented on QPID-572:
-------------------------------------

I too managed to replicate the issue however I had to put a delay in of 1500s and therefore ensure that I upped the receive(...) methods in the TransactedTest. I also edited the TTest so that it used a local broker rather than inVM.

I'll attach the patch that should address the issue. I haven't had time to fully test it so apologies if it doesn't do it. I haven't run the python tests but the only maven time test that failed was TopicSessionTest. but as usual running the test in Intelij works just fine.



> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Rafael H. Schloming (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12527624 ] 

Rafael H. Schloming commented on QPID-572:
------------------------------------------

I believe the original fix for this issue is actually more correct than the modified fix, although the modified fix might be considered a reasonable M2 workaround for QPID-601. Please see my comments on QPID-601 for details.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: QPID-572-testcase.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (QPID-572) broker delivers messages out of order

Posted by "Martin Ritchie (JIRA)" <qp...@incubator.apache.org>.
    [ https://issues.apache.org/jira/browse/QPID-572?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12526370 ] 

Martin Ritchie commented on QPID-572:
-------------------------------------

Ah, that would do it. as nextSubscriber() returns the next subscriber that is able to consume this message. This includes this test

subscription.getPreDeliveryQueue().isEmpty()

Which inessence is the selector. Hence why I suggested we needed the additional subscription.getDeliveryQueue().isEmpty() method. However, it makes more sence to put that check at the top level as it pertains to the queue as a whole. 

I need to write up how the selectors work but the short hand is that each message that is delivered is evaluated once against each selector using subscriber via subscriber.hasInterest() if they are interested then the msgs is added to a PreDelivery Queue that is unique to each subscriber. To prevent two subscribers getting the same message the message now has a notion of taken(). So the getNextMessage() method checks for this and simply removes any taken msgs when looking for the next message to send. Ideally we could have a reaper thread doing some of this but I haven't had time to design that as there are a number of other tasks such a reaper thread should do. TTL being one of them. One of my next tasks is to improve some of the technical documentation so I shall do the CSDM / selector work first to help review the selector changes.

> broker delivers messages out of order
> -------------------------------------
>
>                 Key: QPID-572
>                 URL: https://issues.apache.org/jira/browse/QPID-572
>             Project: Qpid
>          Issue Type: Bug
>          Components: Java Broker
>    Affects Versions: M2, M2.1, M3
>            Reporter: Rafael H. Schloming
>         Attachments: CSDM.patch
>
>
> ConcurrentSelectorDeliveryManager will sometimes deliver messages out of order. This is caused by the code in deliver(...) that attempts to short-circuit message queuing when there is an available subscription. This code can result in the currently published message skipping ahead of queued messages causing out of order delivery. Although unrelated to transactions, I have observed this failure occuring in TransactedTest both in testCommit and testRollback. Normally it does not happen very frequently, however placing a Thread.sleep(500) in the async delivery thread will cause the failure to occur almost all the time.
> I tried fixing the problem by only attempting synchronous delivery when there are no queued messages, however this appears to break other tests that use selectors. This makes me suspect that the selector implementation is somehow incorrectly coupled to synchronous delivery.
> I have only verfied this issue on the trunk, however I believe it effects M2 as well.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.