Qpid
  1. Qpid
  2. QPID-3481

Acquired messages are not sent to alternate exchange when queue is deleted and receiver's session closed

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.15
    • Component/s: C++ Broker
    • Labels:
      None

      Description

      Currently, the broker will lose messages in the following scenario:

      1. Client creates receiver to a queue (queue has an alternate exchange configured)
      2. Client acquires message from queue
      3. Client closes receiver (with delete:always or delete:receiver)
      4. Client closes session

      We expect that the messages should be sent to the now-deleted queue's alternate exchange if/when they are released.

      Messages are not sent to the alternate exchange immediately upon closure of the receiver because the client still has acquired messages; this is fine, but when the client either releases the messages or closes the session, these messages should be sent to the alternate exchange.

      1. QPID-3481.patch
        2 kB
        Ted Ross
      2. QPID-3481.diff
        4 kB
        Andy Goldstein

        Activity

        Hide
        Andy Goldstein added a comment -

        Potential patch

        Show
        Andy Goldstein added a comment - Potential patch
        Hide
        Gordon Sim added a comment -

        Deleting a queue before you have released or accepted any messages from it is an odd thing to do in my opinion. However the 0-10 specification states 'When a queue is deleted any pending messages are sent to the alternate-exchange if defined, or discarded if it is not.'

        The correct fix therefore is to reroute the messages at the point of deletion. My advice would be to avoid deleting a queue in this state if at all possible.

        Show
        Gordon Sim added a comment - Deleting a queue before you have released or accepted any messages from it is an odd thing to do in my opinion. However the 0-10 specification states 'When a queue is deleted any pending messages are sent to the alternate-exchange if defined, or discarded if it is not.' The correct fix therefore is to reroute the messages at the point of deletion. My advice would be to avoid deleting a queue in this state if at all possible.
        Hide
        Andy Goldstein added a comment -

        While I agree that it would be ideal to avoid this situation, there are times in our system where this will happen. We have a high priority bug related to this issue, and would like to see it resolved. As it currently stands, if you acquire a message and then delete the queue, the queue will not have its destructor called until all the DeliveryRecords have been cleaned up. This won't happen until the client's session has been closed. When the session is closed and the destructor is called, messages that were previously acquired are returned to the "deleted" queue, and then the queue is deleted from memory. The patch I attached attempts to fix this gap; namely, it performs the same logic in Queue::destroyed() that routes all messages to the alternate exchange. At least this way, messages are not lost.

        Show
        Andy Goldstein added a comment - While I agree that it would be ideal to avoid this situation, there are times in our system where this will happen. We have a high priority bug related to this issue, and would like to see it resolved. As it currently stands, if you acquire a message and then delete the queue, the queue will not have its destructor called until all the DeliveryRecords have been cleaned up. This won't happen until the client's session has been closed. When the session is closed and the destructor is called, messages that were previously acquired are returned to the "deleted" queue, and then the queue is deleted from memory. The patch I attached attempts to fix this gap; namely, it performs the same logic in Queue::destroyed() that routes all messages to the alternate exchange. At least this way, messages are not lost.
        Hide
        Andy Goldstein added a comment -

        To give a little more context: our application uses a Receiver w/capacity > 0 to pull messages from a queue. These messages are acquired (but not immediately acked), transmitted to an external system, and this transmission may fail. If the transmission fails (exception), we catch it and close the receiver, which deletes the queue. The queue has an alternate exchange, and we need all messages to be routed to it.

        I believe we're modifying our catch block now to release acquired messages prior to closing the receiver, but I don't believe that will fix the message loss issue without some sort of patch.

        Show
        Andy Goldstein added a comment - To give a little more context: our application uses a Receiver w/capacity > 0 to pull messages from a queue. These messages are acquired (but not immediately acked), transmitted to an external system, and this transmission may fail. If the transmission fails (exception), we catch it and close the receiver, which deletes the queue. The queue has an alternate exchange, and we need all messages to be routed to it. I believe we're modifying our catch block now to release acquired messages prior to closing the receiver, but I don't believe that will fix the message loss issue without some sort of patch.
        Hide
        Gordon Sim added a comment -

        Why do you want to delete the queue on closing the receiver (rather than say when the session ends)? Releasing any messages fetched from that queue before the queue is deleted should cause them to be re-routed. They may get immediately redelivered to the client, but as long as they are not fetched the client library will release them when the receiver is closed.

        Show
        Gordon Sim added a comment - Why do you want to delete the queue on closing the receiver (rather than say when the session ends)? Releasing any messages fetched from that queue before the queue is deleted should cause them to be re-routed. They may get immediately redelivered to the client, but as long as they are not fetched the client library will release them when the receiver is closed.
        Hide
        Andy Goldstein added a comment - - edited

        After talking to my developer, what we are going to do is set capacity to 0, session.reject() all acquired messages, close the receiver (thus deleting the queue), and close the session. This should fix our issue.

        Not all of our queues are auto-delete queues, so there are times when we want the closing of the receiver to result in the queue being deleted.

        There is still a small potential for message loss in general - with a queue with delete:always, acquire a message, then close the receiver and session. The message is lost because the DeliveryRecord still exists when the receiver is closed/queue is deleted. When the session is closed, the DeliveryRecord goes away, restoring the message to the (deleted) queue, and then the queue's destructor is called, but the restored message is never sent to the alternate exchange.

        Show
        Andy Goldstein added a comment - - edited After talking to my developer, what we are going to do is set capacity to 0, session.reject() all acquired messages, close the receiver (thus deleting the queue), and close the session. This should fix our issue. Not all of our queues are auto-delete queues, so there are times when we want the closing of the receiver to result in the queue being deleted. There is still a small potential for message loss in general - with a queue with delete:always, acquire a message, then close the receiver and session. The message is lost because the DeliveryRecord still exists when the receiver is closed/queue is deleted. When the session is closed, the DeliveryRecord goes away, restoring the message to the (deleted) queue, and then the queue's destructor is called, but the restored message is never sent to the alternate exchange.
        Hide
        Andy Goldstein added a comment -

        Changing the priority to minor as this is less of an impact to us.

        Show
        Andy Goldstein added a comment - Changing the priority to minor as this is less of an impact to us.
        Hide
        Ted Ross added a comment -

        For the record:

        The reason this happens is that when the client deletes the subscription queue (on receiver/session close), the call to Queue::destroyed occurs before the acquired messages in the DeliveryRecords are re-queued. This means that the acquired messages are re-queued after the queue is emptied into the alternate exchange.

        Show
        Ted Ross added a comment - For the record: The reason this happens is that when the client deletes the subscription queue (on receiver/session close), the call to Queue::destroyed occurs before the acquired messages in the DeliveryRecords are re-queued. This means that the acquired messages are re-queued after the queue is emptied into the alternate exchange.
        Hide
        Ted Ross added a comment -

        I've attached a new patch for this issue.

        The problem was that messages requeued from a subscription into a deleted queue were not re-routed through the alternate exchange.

        This patch fixes that problem.

        Show
        Ted Ross added a comment - I've attached a new patch for this issue. The problem was that messages requeued from a subscription into a deleted queue were not re-routed through the alternate exchange. This patch fixes that problem.
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/
        -----------------------------------------------------------

        Review request for Gordon Sim and Kenneth Giusti.

        Summary
        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.
        https://issues.apache.org/jira/browse/QPID-3481

        Diffs


        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing
        -------

        Passes "make check"

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing ------- Passes "make check" Thanks, Ted
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/
        -----------------------------------------------------------

        (Updated 2012-02-01 19:51:00.204223)

        Review request for Gordon Sim and Kenneth Giusti.

        Summary
        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.
        https://issues.apache.org/jira/browse/QPID-3481

        Diffs


        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing (updated)
        -------

        Passes "make check"
        The fix to QPID-3481 has been verified.

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- (Updated 2012-02-01 19:51:00.204223) Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing (updated) ------- Passes "make check" The fix to QPID-3481 has been verified. Thanks, Ted
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/#review4757
        -----------------------------------------------------------

        Ship it!

        One minor question (see below) - otherwise looks fine.

        trunk/qpid/cpp/src/qpid/broker/Queue.cpp
        <https://reviews.apache.org/r/3724/#comment10484>

        If deleted, would invoking populate here result in (other) consumers being scheduled to consume from this queue although the message have not been requeued to it?

        • Kenneth

        On 2012-02-01 19:51:00, Ted Ross wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3724/

        -----------------------------------------------------------

        (Updated 2012-02-01 19:51:00)

        Review request for Gordon Sim and Kenneth Giusti.

        Summary

        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.

        https://issues.apache.org/jira/browse/QPID-3481

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing

        -------

        Passes "make check"

        The fix to QPID-3481 has been verified.

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/#review4757 ----------------------------------------------------------- Ship it! One minor question (see below) - otherwise looks fine. trunk/qpid/cpp/src/qpid/broker/Queue.cpp < https://reviews.apache.org/r/3724/#comment10484 > If deleted, would invoking populate here result in (other) consumers being scheduled to consume from this queue although the message have not been requeued to it? Kenneth On 2012-02-01 19:51:00, Ted Ross wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- (Updated 2012-02-01 19:51:00) Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs ----- trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing ------- Passes "make check" The fix to QPID-3481 has been verified. Thanks, Ted
        Hide
        jiraposter@reviews.apache.org added a comment -

        On 2012-02-01 20:47:34, Kenneth Giusti wrote:

        > trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 238

        > <https://reviews.apache.org/r/3724/diff/1/?file=71667#file71667line238>

        >

        > If deleted, would invoking populate here result in (other) consumers being scheduled to consume from this queue although the message have not been requeued to it?

        >

        There's probably no harm in poking the other consumers since they couldn't all get the message anyway. That said, I think it's more correct to not call populate in the deleted case. I'll update the patch as soon as I re-run the tests.

        • Ted

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/#review4757
        -----------------------------------------------------------

        On 2012-02-01 19:51:00, Ted Ross wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3724/

        -----------------------------------------------------------

        (Updated 2012-02-01 19:51:00)

        Review request for Gordon Sim and Kenneth Giusti.

        Summary

        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.

        https://issues.apache.org/jira/browse/QPID-3481

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing

        -------

        Passes "make check"

        The fix to QPID-3481 has been verified.

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - On 2012-02-01 20:47:34, Kenneth Giusti wrote: > trunk/qpid/cpp/src/qpid/broker/Queue.cpp, line 238 > < https://reviews.apache.org/r/3724/diff/1/?file=71667#file71667line238 > > > If deleted, would invoking populate here result in (other) consumers being scheduled to consume from this queue although the message have not been requeued to it? > There's probably no harm in poking the other consumers since they couldn't all get the message anyway. That said, I think it's more correct to not call populate in the deleted case. I'll update the patch as soon as I re-run the tests. Ted ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/#review4757 ----------------------------------------------------------- On 2012-02-01 19:51:00, Ted Ross wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- (Updated 2012-02-01 19:51:00) Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs ----- trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing ------- Passes "make check" The fix to QPID-3481 has been verified. Thanks, Ted
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/
        -----------------------------------------------------------

        (Updated 2012-02-01 22:03:02.070364)

        Review request for Gordon Sim and Kenneth Giusti.

        Summary
        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.
        https://issues.apache.org/jira/browse/QPID-3481

        Diffs (updated)


        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing
        -------

        Passes "make check"
        The fix to QPID-3481 has been verified.

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- (Updated 2012-02-01 22:03:02.070364) Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs (updated) trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing ------- Passes "make check" The fix to QPID-3481 has been verified. Thanks, Ted
        Hide
        jiraposter@reviews.apache.org added a comment -

        -----------------------------------------------------------
        This is an automatically generated e-mail. To reply, visit:
        https://reviews.apache.org/r/3724/#review4897
        -----------------------------------------------------------

        I'm not hugely keen on this fix. While by no mean horrendous, it does convolute the code. I think it likely that we will shortly have a modified Queue that retains acquired messages at which point a cleaner - and more logically consistent - fix is possible. If you do commit this, it would be good to have a test case with it.

        • Gordon

        On 2012-02-01 22:03:02, Ted Ross wrote:

        -----------------------------------------------------------

        This is an automatically generated e-mail. To reply, visit:

        https://reviews.apache.org/r/3724/

        -----------------------------------------------------------

        (Updated 2012-02-01 22:03:02)

        Review request for Gordon Sim and Kenneth Giusti.

        Summary

        -------

        This patch makes one change that fixes the reported bug:

        Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted.

        This addresses bug QPID-3481.

        https://issues.apache.org/jira/browse/QPID-3481

        Diffs

        -----

        trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252

        Diff: https://reviews.apache.org/r/3724/diff

        Testing

        -------

        Passes "make check"

        The fix to QPID-3481 has been verified.

        Thanks,

        Ted

        Show
        jiraposter@reviews.apache.org added a comment - ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/#review4897 ----------------------------------------------------------- I'm not hugely keen on this fix. While by no mean horrendous, it does convolute the code. I think it likely that we will shortly have a modified Queue that retains acquired messages at which point a cleaner - and more logically consistent - fix is possible. If you do commit this, it would be good to have a test case with it. Gordon On 2012-02-01 22:03:02, Ted Ross wrote: ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3724/ ----------------------------------------------------------- (Updated 2012-02-01 22:03:02) Review request for Gordon Sim and Kenneth Giusti. Summary ------- This patch makes one change that fixes the reported bug: Messages that are requeued from a subscription (due to closing or release) to a deleted queue are routed through that queue's alternate exchange if one was configured. Previously, requeued messages were placed back on the queue regardless of whether it had been deleted. This addresses bug QPID-3481 . https://issues.apache.org/jira/browse/QPID-3481 Diffs ----- trunk/qpid/cpp/src/qpid/broker/Queue.cpp 1239252 Diff: https://reviews.apache.org/r/3724/diff Testing ------- Passes "make check" The fix to QPID-3481 has been verified. Thanks, Ted

          People

          • Assignee:
            Ted Ross
            Reporter:
            Andy Goldstein
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development