Qpid
  1. Qpid
  2. QPID-3605

Durable subscriber with no-local true receives messages on re-connection

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.15
    • Fix Version/s: 0.15
    • Component/s: Java Broker
    • Labels:
      None

      Description

      If a durable subscriber using no-local true reconnects to the Java Broker, it receives all messages that publisher on same connection previously sent. This defect is present on all code paths (0-8..0-10)

      See tests:
      (Java) org.apache.qpid.test.unit.topic.DurableSubscriptionTest#testNoLocalOnConnection
      (Python) qpid_tests.broker_0_10.message.MessageTests.test_no_local_awkward

      This defect does not manifest itself on the CPP Broker, as the CPP broker drops these messages.

        Activity

        Hide
        ASF subversion and git services added a comment -

        Commit 1643264 from Keith Wall in branch 'qpid/trunk'
        [ https://svn.apache.org/r1643264 ]

        NO-JIRA: Remove exclusion related to QPID-3605. Looks like this was fixed long ago.

        Show
        ASF subversion and git services added a comment - Commit 1643264 from Keith Wall in branch 'qpid/trunk' [ https://svn.apache.org/r1643264 ] NO-JIRA: Remove exclusion related to QPID-3605 . Looks like this was fixed long ago.
        Hide
        Robbie Gemmell added a comment -

        Looks good, resolving. (Ok, I actually noticed a problem with a comment in the original test now being inaccurate, but it seemed quicker jsut to fix it myself )

        Show
        Robbie Gemmell added a comment - Looks good, resolving. (Ok, I actually noticed a problem with a comment in the original test now being inaccurate, but it seemed quicker jsut to fix it myself )
        Hide
        Rob Godfrey added a comment -

        Robbie - believe your comments are addressed now...

        Show
        Rob Godfrey added a comment - Robbie - believe your comments are addressed now...
        Hide
        Rob Godfrey added a comment -

        Agreed on the naming...

        On the test... I did consider doing that at the time and then got lazy I shall add now ...

        Now there is the "fun" case that if you create a durable sub on connection A, send messages from connection B, close connection A, resubscribe on Connection B then consume... you won't see the messages (due to no-local at the sub) but they will be on the queue... and if you close connection B and then resubscribe on connection C, you will get them... I think the C++ broker behaviour will differ here.

        Essentially the Java broker is going to treat no-local as "if, at the time the message was published, the publisher and subscriber were not the same, then queue the message. Also for any messages in the queue, do not deliver to the connection if the message was published on the connection". The C++, in contrast, appears to say at the queue "if the message is the from the same connection as my current subscriber... then throw the message away".

        Show
        Rob Godfrey added a comment - Agreed on the naming... On the test... I did consider doing that at the time and then got lazy I shall add now ... Now there is the "fun" case that if you create a durable sub on connection A, send messages from connection B, close connection A, resubscribe on Connection B then consume... you won't see the messages (due to no-local at the sub) but they will be on the queue... and if you close connection B and then resubscribe on connection C, you will get them... I think the C++ broker behaviour will differ here. Essentially the Java broker is going to treat no-local as "if, at the time the message was published, the publisher and subscriber were not the same, then queue the message. Also for any messages in the queue, do not deliver to the connection if the message was published on the connection". The C++, in contrast, appears to say at the queue "if the message is the from the same connection as my current subscriber... then throw the message away".
        Hide
        Robbie Gemmell added a comment -

        TopicExchange.createSelectorFilter() now does more than creating selector filter, it should be renamed for clarity. There are a load of if statements without any braces.

        I think it would be good if the test published some messages on a different connection that the no-local subscriber could get but deliberately doesnt retrieve before the broker restart, just to ensure they do get delivered after the restart and reconnection.

        As discussed in person earlier, the additional then removal of no-local (or vice versa) will interact with the selector change verification code and provoke an unsubscribe (queue-delete) at reconnect, but we think that is reaosnable enough.

        Show
        Robbie Gemmell added a comment - TopicExchange.createSelectorFilter() now does more than creating selector filter, it should be renamed for clarity. There are a load of if statements without any braces. I think it would be good if the test published some messages on a different connection that the no-local subscriber could get but deliberately doesnt retrieve before the broker restart, just to ensure they do get delivered after the restart and reconnection. As discussed in person earlier, the additional then removal of no-local (or vice versa) will interact with the selector change verification code and provoke an unsubscribe (queue-delete) at reconnect, but we think that is reaosnable enough.
        Hide
        Rob Godfrey added a comment -

        Robbie - could you review - thx

        Show
        Rob Godfrey added a comment - Robbie - could you review - thx
        Hide
        Rob Godfrey added a comment -

        I've updated the client/broker so that the binding between the queue and the exchange contains the argument x-qpid-no-local. The topic exchange treats this as a special type of filter, excluding messages which have arrived on the same connection that "owns" the queue

        Show
        Rob Godfrey added a comment - I've updated the client/broker so that the binding between the queue and the exchange contains the argument x-qpid-no-local. The topic exchange treats this as a special type of filter, excluding messages which have arrived on the same connection that "owns" the queue
        Hide
        Robbie Gemmell added a comment -

        When undertaking QPID-3829 the changes made also stopped an NPE on the 0-8/0-9/0-9-1 subscriptions when evaluating no-local after store recovery, allowing NoLocalAfterRecoveryTest to be enabled again (though updated to make it simpler and more reliable.) If this JIRA is undertaken, that test will need updated to ensure that messages the client should have got (then excluding the ones that wouldnt have matched the no-local check when they were actually publsihed) but didnt before the broker restarted were successfully delivered afer a new no-local connection was established.

        Show
        Robbie Gemmell added a comment - When undertaking QPID-3829 the changes made also stopped an NPE on the 0-8/0-9/0-9-1 subscriptions when evaluating no-local after store recovery, allowing NoLocalAfterRecoveryTest to be enabled again (though updated to make it simpler and more reliable.) If this JIRA is undertaken, that test will need updated to ensure that messages the client should have got (then excluding the ones that wouldnt have matched the no-local check when they were actually publsihed) but didnt before the broker restarted were successfully delivered afer a new no-local connection was established.

          People

          • Assignee:
            Robbie Gemmell
            Reporter:
            Keith Wall
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development