Qpid
  1. Qpid
  2. QPID-4521

[Java broker] routing to an alternate exchange during queue deletion and AMQP 0-10 message-reject fails when it is a topic exchange

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.16, 0.18, 0.20
    • Fix Version/s: 0.20
    • Component/s: Java Broker
    • Labels:
      None

      Description

      Routing to an alternate exchange during queue deletion fails for topic exchanges. This is because the InboundMessageAdapter used to adapt the queues QueueEntry for routing by the exchange has a defect in the getRountingKeyShortSring() method. This method returns an AMQShortString and is used by the Topic exchange (to avoid redoing much of the Topic matching logic that uses AMQShortString) whereas the other exchanges use the getRoutingKey() method which returns a String.

        Activity

        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open In Progress In Progress
        18m 7s 1 Robbie Gemmell 23/Dec/12 19:58
        In Progress In Progress Reviewable Reviewable
        2s 1 Robbie Gemmell 23/Dec/12 19:58
        Reviewable Reviewable Resolved Resolved
        22h 1 Keith Wall 24/Dec/12 17:58
        Resolved Resolved Closed Closed
        779d 2h 9m 1 Rob Godfrey 11/Feb/15 20:07
        Rob Godfrey made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Hide
        Robbie Gemmell added a comment -

        Now merged to the 0.20 release branch.

        Show
        Robbie Gemmell added a comment - Now merged to the 0.20 release branch.
        Robbie Gemmell made changes -
        Fix Version/s 0.20 [ 12323548 ]
        Fix Version/s 0.21 [ 12323549 ]
        Hide
        Justin Ross added a comment -

        Reviewed by Keith. Approved for 0.20.

        Show
        Justin Ross added a comment - Reviewed by Keith. Approved for 0.20.
        Robbie Gemmell made changes -
        Summary [Java broker] routing to an alternate exchange during queue deletion fails for topic exchanges. [Java broker] routing to an alternate exchange during queue deletion and AMQP 0-10 message-reject fails when it is a topic exchange
        Hide
        Robbie Gemmell added a comment -

        It hadn't actually occurred to me that method would even be relevant here, as it isn't used during queue deletion...but since it will be doing something incredibly similar (potentially to the point it should be updated to allow use within the queue deletion process, at which point also adding some debug logging might be useful yes) that means rejecting individual messages via AMQP 0-10 would also be affected by this defect. Updating JIRA title accordingly.

        Show
        Robbie Gemmell added a comment - It hadn't actually occurred to me that method would even be relevant here, as it isn't used during queue deletion...but since it will be doing something incredibly similar (potentially to the point it should be updated to allow use within the queue deletion process, at which point also adding some debug logging might be useful yes) that means rejecting individual messages via AMQP 0-10 would also be affected by this defect. Updating JIRA title accordingly.
        Keith Wall made changes -
        Status Ready To Review [ 10006 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Hide
        Keith Wall added a comment -

        Hi Robbie,

        I've reviewed your change and I have no comments. I did wonder if we ought to have a little more debug logging around QueueEntryImpl.routeToAlternate() might be helpful?

        Perhaps a debug logging statement to log - if the message has been routed to a queue(s) on the alternate exchange, or queues on the alternate's alternate exchange, or nowhere at all.

        What do you think?

        Show
        Keith Wall added a comment - Hi Robbie, I've reviewed your change and I have no comments. I did wonder if we ought to have a little more debug logging around QueueEntryImpl.routeToAlternate() might be helpful? Perhaps a debug logging statement to log - if the message has been routed to a queue(s) on the alternate exchange, or queues on the alternate's alternate exchange, or nowhere at all. What do you think?
        Robbie Gemmell made changes -
        Assignee Robbie Gemmell [ gemmellr ] Keith Wall [ k-wall ]
        Hide
        Robbie Gemmell added a comment -

        Keith, can you review please?

        I'd like to request this goes into 0.20 if possible, since it is so isolated and trivial.

        Show
        Robbie Gemmell added a comment - Keith, can you review please? I'd like to request this goes into 0.20 if possible, since it is so isolated and trivial.
        Robbie Gemmell made changes -
        Status In Progress [ 3 ] Ready To Review [ 10006 ]
        Robbie Gemmell made changes -
        Field Original Value New Value
        Status Open [ 1 ] In Progress [ 3 ]
        Hide
        Robbie Gemmell added a comment -

        Change committed in http://svn.apache.org/viewvc?rev=1425515&view=rev which corrects the defect and adds unit tests for the InboundMessageAdapter to verify it is returning the correct values.

        I didn't add any system test using an alternate topic exchange (although I did actually write such a test to initially confirm the defect) as both the topic exchange itself and the alternate exchange functionality are already system tested quite heavily (mostly in the python tests but a couple in Java, its just that they use Fanout exchanges for the alternate exchanges, which is a far more typical use case) and this is simply a trivial defect in a piece of code that works between the two.

        Show
        Robbie Gemmell added a comment - Change committed in http://svn.apache.org/viewvc?rev=1425515&view=rev which corrects the defect and adds unit tests for the InboundMessageAdapter to verify it is returning the correct values. I didn't add any system test using an alternate topic exchange (although I did actually write such a test to initially confirm the defect) as both the topic exchange itself and the alternate exchange functionality are already system tested quite heavily (mostly in the python tests but a couple in Java, its just that they use Fanout exchanges for the alternate exchanges, which is a far more typical use case) and this is simply a trivial defect in a piece of code that works between the two.
        Robbie Gemmell created issue -

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development