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

        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.
        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.
        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.
        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?
        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.
        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.

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development