Uploaded image for project: 'Qpid'
  1. Qpid
  2. QPID-4164

management move/copy and max delivery attempt functionality fail to properly move/copy the message if it was loaded from the message store at startup

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 0.16
    • Fix Version/s: 0.18
    • Component/s: Broker-J

      Description

      The JMX moveMessage/copyMessage functions fail to properly move/copy the message if the message has been recovered from the store at Broker startup. The Broker permanently loses the message's payload.

      If an operator subsequently tries to view the problematic message with the Management interfaces, a JMX thread will be put into an infinite loop (see QPID-4170) and this will require a Broker restart.

      This problem does not affect messages sent during the lifetime of the Broker's execution (they can be moved or copied as normal). The existence of messages with lost payload within the Broker does not affect the flight of other messages through the Broker.

        Activity

        Hide
        k-wall Keith Wall added a comment -

        Patch applied, Phil/Robbie could you review this change please?

        Show
        k-wall Keith Wall added a comment - Patch applied, Phil/Robbie could you review this change please?
        Hide
        gemmellr Robbie Gemmell added a comment -

        The changes look good to me except one small thing: in the Derby store it used to only keep the hard reference to the metadata if it still needed to be persisted whereas after the change it always does it, so recovered messages will always hard reference the metadata. I have restored the previous behaviour to the Derby store and added the same behaviour to the BDB store, allowing the SoftReference to release the data the same way it can for new messages arriving off the wire (if memory pressure demands it).

        Can you review my changes?

        (I would personally consider this to be a 0.18 blocker)

        Show
        gemmellr Robbie Gemmell added a comment - The changes look good to me except one small thing: in the Derby store it used to only keep the hard reference to the metadata if it still needed to be persisted whereas after the change it always does it, so recovered messages will always hard reference the metadata. I have restored the previous behaviour to the Derby store and added the same behaviour to the BDB store, allowing the SoftReference to release the data the same way it can for new messages arriving off the wire (if memory pressure demands it). Can you review my changes? (I would personally consider this to be a 0.18 blocker)
        Hide
        philharveyonline Philip Harvey added a comment -

        This issue also affected messages that the broker moved to a DLQ, so a test should be added to check that this is fixed.

        Show
        philharveyonline Philip Harvey added a comment - This issue also affected messages that the broker moved to a DLQ, so a test should be added to check that this is fixed.
        Hide
        philharveyonline Philip Harvey added a comment -

        enhance MaxDeliveryCountTest to check that this problem no longer affects DLQs

        Show
        philharveyonline Philip Harvey added a comment - enhance MaxDeliveryCountTest to check that this problem no longer affects DLQs
        Hide
        philharveyonline Philip Harvey added a comment -

        attached patch to extend MaxDeliveryCountTest to check for this bug - please review and commit if you're happy.

        Show
        philharveyonline Philip Harvey added a comment - attached patch to extend MaxDeliveryCountTest to check for this bug - please review and commit if you're happy.
        Hide
        k-wall Keith Wall added a comment -

        Hi Robbie, I reviewed your change of 27/Jul/12 11:28. No comments. I agree these commits should be part of 0.18.

        Show
        k-wall Keith Wall added a comment - Hi Robbie, I reviewed your change of 27/Jul/12 11:28. No comments. I agree these commits should be part of 0.18.
        Hide
        k-wall Keith Wall added a comment -

        Hi Phil, no comments on your patch. It has been applied to trunk.

        Show
        k-wall Keith Wall added a comment - Hi Phil, no comments on your patch. It has been applied to trunk.
        Hide
        justi9 Justin Ross added a comment -

        Reviewed by Keith, Philip, and Robbie. Approved for 0.18.

        Show
        justi9 Justin Ross added a comment - Reviewed by Keith, Philip, and Robbie. Approved for 0.18.
        Hide
        gemmellr Robbie Gemmell added a comment -

        Changes merged to the 0.18 release branch.

        Show
        gemmellr Robbie Gemmell added a comment - Changes merged to the 0.18 release branch.
        Hide
        gemmellr Robbie Gemmell added a comment -

        Attaching a patch for 0.16, should anyone need to resolve this issue for an existing system.

        It is slightly different than the trunk/0.18 changes in that it doesn't contain the metadata reference optimisation I mentioned above (to keep the change as isolated to the actual defect as possible) and lacks some of the tests added on trunk (but has enough to expose the issue and see that it is resolved).

        Show
        gemmellr Robbie Gemmell added a comment - Attaching a patch for 0.16, should anyone need to resolve this issue for an existing system. It is slightly different than the trunk/0.18 changes in that it doesn't contain the metadata reference optimisation I mentioned above (to keep the change as isolated to the actual defect as possible) and lacks some of the tests added on trunk (but has enough to expose the issue and see that it is resolved).

          People

          • Assignee:
            gemmellr Robbie Gemmell
            Reporter:
            k-wall Keith Wall
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development