Qpid
  1. Qpid
  2. QPID-3157

'removed' subscriptions may be held in memory by the queues SubscriptionList or 'lastSubscriptionNode' reference

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.5, 0.6, 0.7, 0.8, 0.9, 0.10, 0.11, 0.12
    • Fix Version/s: 0.13
    • Component/s: Java Broker
    • Labels:
      None

      Description

      subscriptions 'removed' from a queue subscription list are only marked deleted and can not actually bereleased from memory until the head of the subscription list advances beyond them. Additionally and perhaps more troublesome, a queues 'lastSubscriptionNode' can refer to a particular subscription but hold all subscequent subscriptions in memory whether they have been deleted or not and regardless whether the head of the SubscriptionList has advanced beyond them,

      As a result any memory in use by the now-closed subscription will not be released until the queue is deleted, or all the subscriptions prior to it are closed and removed from the list and the 'lastSubscriptionNode' advances beyond them.. This also holds the associated ServerSession in memory, which is currently a rather heavyweight object.

      This issue is further compounded when the queue has a backlog of messages and consumers are then created, used to recieve one message, and closed. In this scenario the broker sends the client as many messages as it can prefetch, leading to creation of up to <prefetch size, default=500> MessageTransfer commands, all which are recorded in the ServerSession but then left retained as 'unprocessed' in the closed session, which might be held in memory as described above. This results in an explosion in the number of MessageTransfer commands retained in memory as each message can have up to <prefetch size> MessageTransfer commands associated with it before it is eventually consumed by a client and all of thse will also be retained in memory by a 'removed' but not deleted subscription. The last effect can be combated by restricting the preftech size (eg appending &maxprefetch='1' to the connection url).

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Reviewable Reviewable Open Open
          2h 51m 1 Robbie Gemmell 30/Jul/11 19:10
          Open Open In Progress In Progress
          132d 15h 50m 2 Robbie Gemmell 30/Jul/11 19:10
          In Progress In Progress Reviewable Reviewable
          1d 2h 46m 2 Robbie Gemmell 31/Jul/11 21:57
          Reviewable Reviewable Resolved Resolved
          1d 13h 50m 1 Keith Wall 02/Aug/11 11:48
          Resolved Resolved Closed Closed
          1289d 9h 19m 1 Rob Godfrey 11/Feb/15 20:07
          Rob Godfrey made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Keith Wall made changes -
          Status Ready To Review [ 10006 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Hide
          Keith Wall added a comment -

          Reviewed, no comments.

          Show
          Keith Wall added a comment - Reviewed, no comments.
          Robbie Gemmell made changes -
          Assignee Robbie Gemmell [ gemmellr ] Keith Wall [ k-wall ]
          Hide
          Robbie Gemmell added a comment -

          Added a test to pick up the problem, then fixed up the issue, which was: the marker cleanup could run off the end of the list if the previously-tail node was being removed, and so would end up pointing a dummy marker nodes 'next' reference at null instead of the new list tail, meaning it then wouldnt pick up future Subscriptions added to the list, which would cause the queue to cycle back to the list head preumaturely.

          Back over to you Keith.

          Show
          Robbie Gemmell added a comment - Added a test to pick up the problem, then fixed up the issue, which was: the marker cleanup could run off the end of the list if the previously-tail node was being removed, and so would end up pointing a dummy marker nodes 'next' reference at null instead of the new list tail, meaning it then wouldnt pick up future Subscriptions added to the list, which would cause the queue to cycle back to the list head preumaturely. Back over to you Keith.
          Robbie Gemmell made changes -
          Status In Progress [ 3 ] Ready To Review [ 10006 ]
          Robbie Gemmell made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Robbie Gemmell made changes -
          Status Ready To Review [ 10006 ] Open [ 1 ]
          Assignee Keith Wall [ k-wall ] Robbie Gemmell [ gemmellr ]
          Hide
          Robbie Gemmell added a comment -

          Bumping this back to In Progress because I just thought of a corner case I didnt cover when cleaning up the marker node, will fix it up soon.

          Show
          Robbie Gemmell added a comment - Bumping this back to In Progress because I just thought of a corner case I didnt cover when cleaning up the marker node, will fix it up soon.
          Hide
          Robbie Gemmell added a comment -

          Just a note, QPID-3319 should be closed out at the same time as this is (without a fix-for; no point having 2 JIRAs for the same issue in the release notes).

          Show
          Robbie Gemmell added a comment - Just a note, QPID-3319 should be closed out at the same time as this is (without a fix-for; no point having 2 JIRAs for the same issue in the release notes).
          Robbie Gemmell made changes -
          Assignee Robbie Gemmell [ gemmellr ] Keith Wall [ k-wall ]
          Hide
          Robbie Gemmell added a comment -

          Hi Keith, could you review these changes please?

          The SubscriptionList now ensures it removes the nodes during the remove method rather than waiting for the list head to advance past them, which may not occur if earlier Subscription nodes in the list remain active. A dummy node is inserted at the end when attempting to remove the current tail node, as the tail cant be unlinked for thread safety. The new 'marked node' functionlity within the SubscriptionList allows cleaning up any nodes the prior 'lastSubscriptionNode' reference in the queue would have leaked by simply searching for the next non-deleted node after the marker, since this is guaranteed to be within the active list, after which point they are no longer leaked due to the above changes.

          Thanks,
          Robbie.

          Show
          Robbie Gemmell added a comment - Hi Keith, could you review these changes please? The SubscriptionList now ensures it removes the nodes during the remove method rather than waiting for the list head to advance past them, which may not occur if earlier Subscription nodes in the list remain active. A dummy node is inserted at the end when attempting to remove the current tail node, as the tail cant be unlinked for thread safety. The new 'marked node' functionlity within the SubscriptionList allows cleaning up any nodes the prior 'lastSubscriptionNode' reference in the queue would have leaked by simply searching for the next non-deleted node after the marker, since this is guaranteed to be within the active list, after which point they are no longer leaked due to the above changes. Thanks, Robbie.
          Robbie Gemmell made changes -
          Status In Progress [ 3 ] Ready To Review [ 10006 ]
          Robbie Gemmell made changes -
          Status Open [ 1 ] In Progress [ 3 ]
          Robbie Gemmell made changes -
          Fix Version/s 0.13 [ 12316854 ]
          Fix Version/s 0.11 [ 12316272 ]
          Affects Version/s 0.11 [ 12316272 ]
          Affects Version/s 0.12 [ 12316848 ]
          Robbie Gemmell made changes -
          Link This issue is related to QPID-3319 [ QPID-3319 ]
          Robbie Gemmell made changes -
          Priority Blocker [ 1 ] Critical [ 2 ]
          Robbie Gemmell made changes -
          Assignee Robbie Gemmell [ gemmellr ]
          Robbie Gemmell made changes -
          Summary subscriptions 'removed' from a queue subscription list are only marked deleted and not actually completely removed from the list 'removed' subscriptions may be held in memory by the queues SubscriptionList or 'lastSubscriptionNode' reference
          Fix Version/s 0.11 [ 12316272 ]
          Fix Version/s 0.10 [ 12316273 ]
          Affects Version/s 0.10 [ 12316273 ]
          Description subscriptions 'removed' from a queue subscription list are only marked deleted and not actually released from memory. As a result any memory in use by the now-closed subscription will not be released until the queue is deleted. This also holds the ServerSession in memory.

          This issue compounded when the queue has a backlog of messages and consumers are created, used to recieve one message, and then closed. In this scenario the broker sends the client as many messages as it can prefetch, leading to creation of up to <prefetch size, default=500> MessageTransfer commands, all which are recorded in the ServerSession but then left retained as 'unprocessed' in the closed session. This results in an explosion in the number of MessageTransfer commands retained in memory as each message can have up to <prefetch size> MessageTransfer commands associated with it before it is eventually consumed by a client and all of thse will also be retained in memory. The last effect can be combated by restricting the preftech size (eg appending &maxprefetch='1' to the connection url).
          subscriptions 'removed' from a queue subscription list are only marked deleted and can not actually bereleased from memory until the head of the subscription list advances beyond them. Additionally and perhaps more troublesome, a queues 'lastSubscriptionNode' can refer to a particular subscription but hold *all* subscequent subscriptions in memory whether they have been deleted or not and regardless whether the head of the SubscriptionList has advanced beyond them,


          As a result any memory in use by the now-closed subscription will not be released until the queue is deleted, or all the subscriptions prior to it are closed and removed from the list *and* the 'lastSubscriptionNode' advances beyond them.. This also holds the associated ServerSession in memory, which is currently a rather heavyweight object.

          This issue is further compounded when the queue has a backlog of messages and consumers are then created, used to recieve one message, and closed. In this scenario the broker sends the client as many messages as it can prefetch, leading to creation of up to <prefetch size, default=500> MessageTransfer commands, all which are recorded in the ServerSession but then left retained as 'unprocessed' in the closed session, which might be held in memory as described above. This results in an explosion in the number of MessageTransfer commands retained in memory as each message can have up to <prefetch size> MessageTransfer commands associated with it before it is eventually consumed by a client and all of thse will also be retained in memory by a 'removed' but not deleted subscription. The last effect can be combated by restricting the preftech size (eg appending &maxprefetch='1' to the connection url).
          Hide
          Robbie Gemmell added a comment -

          Updating this with additional details from further analysis, and bumping out to 0.11: none of these issues are new to 0.10 (its been this way since time began apparently) and the number of aspects involved here make this too sensitive to introduce changes to so late in the 0.10 release process.

          Show
          Robbie Gemmell added a comment - Updating this with additional details from further analysis, and bumping out to 0.11: none of these issues are new to 0.10 (its been this way since time began apparently) and the number of aspects involved here make this too sensitive to introduce changes to so late in the 0.10 release process.
          Robbie Gemmell made changes -
          Fix Version/s 0.10 [ 12316273 ]
          Fix Version/s 0.11 [ 12316272 ]
          Affects Version/s 0.10 [ 12316273 ]
          Robbie Gemmell made changes -
          Field Original Value New Value
          Priority Critical [ 2 ] Blocker [ 1 ]
          Description subscriptions 'removed' from a queue subscription list are only marked deleted and not actually released from memory. As a reult any memory in use by the now-closed subscription will not be released until the queue is deleted. subscriptions 'removed' from a queue subscription list are only marked deleted and not actually released from memory. As a result any memory in use by the now-closed subscription will not be released until the queue is deleted. This also holds the ServerSession in memory.

          This issue compounded when the queue has a backlog of messages and consumers are created, used to recieve one message, and then closed. In this scenario the broker sends the client as many messages as it can prefetch, leading to creation of up to <prefetch size, default=500> MessageTransfer commands, all which are recorded in the ServerSession but then left retained as 'unprocessed' in the closed session. This results in an explosion in the number of MessageTransfer commands retained in memory as each message can have up to <prefetch size> MessageTransfer commands associated with it before it is eventually consumed by a client and all of thse will also be retained in memory. The last effect can be combated by restricting the preftech size (eg appending &maxprefetch='1' to the connection url).
          Robbie Gemmell created issue -

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development