qpid-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robbie Gemmell <robbie.gemm...@gmail.com>
Subject Re: Accumulation of Links from global shared durable subscriptions (JMS 2.0)
Date Tue, 07 Nov 2017 09:51:08 GMT
On 6 November 2017 at 14:42, Lorenz Quack <quack.lorenz@gmail.com> wrote:
> On Mon, 2017-11-06 at 11:07 +0000, Lorenz Quack wrote:
>> On Mon, 2017-11-06 at 10:14 +0000, Robbie Gemmell wrote:
>> >
>> > On 3 November 2017 at 16:58, Rob Godfrey <rob.j.godfrey@gmail.com> wrote:
>> > >
>> > >
>> > > On 1 November 2017 at 10:16, Lorenz Quack <quack.lorenz@gmail.com>
wrote:
>> > >
>> > > >
>> > > >
>> > > > Hi,
>> > > >
>> > > > We noticed a potential problem with the way JMS 2.0 global shared
>> > > > durable subscriptions are implemented in the JMS client.  The
>> > > > implementation was designed, discussed, and implemented in QPIDJMS-220
>> > > > [1].
>> > > >
>> > > > When a consumer of a subscription closes the underlying AMQP link
is
>> > > > not closed but merely detached since the closing of a subscription
>> > > > link is used as the signal to unsubscribe the subscription.  These
>> > > > subscription links remain on the broker until the subscription is
>> > > > unsubscribed (detach with close=true) at which point the broker
>> > > > discards all links associated with the subscription.
>> > > >
>> > > > For non-global subscriptions this does not seem to be a problem since
>> > > > the clientId is used as the container-id and links are reused if
>> > > > possible.
>> > > >
>> > > > For global subscription on the other hand the client uses a random
>> > > > UUID associated with the connection as its own container-id.  This
>> > > > means that on every new connection a new link is being estblished
>> > > > rather than recovering an existing one.  Over time these subscription
>> > > > links would accumulate on the broker until the subscription is
>> > > > unsubscribed.
>> > > >
>> > > > Since shared durable subscriptions are expected to be long lived this
>> > > > raises some concerns with regards to resource exhaustion on the
>> > > > broker.  An application that would spin up a connection do some work
>> > > > on a shared durable global subscription and then disconnect again
>> > > > would be "leaking" links.
>> > > >
>> > > > Due to this concern we plan on disabling shared durable global
>> > > > subscriptions for the initial v7 release of Qpid Broker-J.  There
will
>> > > > be a context variable to enable the feature.
>> > > >
>> > > >
>> > > It seems to me a real shame that we are disabling this feature of JMS 2.0
>> > > because of the fact that from an AMQP point of view the link *may* be
>> > > resumed, but in practice, with JMS, will never be.  Would it not be better
>> > > to have a context variable which affects the behaviour of such links to
>> > > shared durable subscriptions such that if the context variable returns
>> > > "true" the links are removed from the link registry at the time of
>> > > connection closure, and if the context value is false, then the current
>> > > behaviour is maintained.  We could then later add functionality to the
>> > > client to specify (via some flag) the behaviour it desires (thus ultimately
>> > > removing the need for the context variable)?
>> > >
>> > > -- Rob
>> > >
>> > >
>> > > >
>> > > >
>> > > >
>> > > > Thoughts, comments, discuss!
>> > > >
>> > > > Kind regards,
>> > > > Lorenz
>> > > >
>> > > >
>> > > > [1] https://issues.apache.org/jira/browse/QPIDJMS-220
>> > > >
>> > I agree that disabling-by-default one of the more useful feature
>> > additions seems a real shame. The timing was dissapointing also.
>> >
>> > The alternative idea for the context variable sounds good to me, as
>> > does having the client flag its behaviour in some way.
>> >
>> > Robbie
>>
>> Thanks for all the feedback.
>> We are looking into options to make this work without leaking Links.
>> Depending on how impactful/intrusive the change would have to be we
>> might consider including this in v7.0.0 since Rob and Robbie seem to
>> feel quite strongly about this.
>>
>> Regarding the timing, we felt that shipping the broker with a known
>> memory leak was undesirable and since this is not a regression but
>> a new feature, disabling the new feature by default until a proper
>> solution (with a potential change to QPIDJMS-220 and the client)
>> could be devised seemed like a reasonable way forward.  I am sorry
>> that you feel differently.  I hope we can come up with a solution
>> that is acceptable to everyone.
>>
>>
>> Kind regards,
>> Lorenz
>>
>
> Hi,
>
> I discussed the above with Keith and Alex and we decided that we
> are willing to change this in v7.0.0 and spin another RC.
>
> I attached a patch to QPID-7998 [1] with our proposed change.
> Below is a description of the patch.  Please indicate whether
> you find these changes acceptable so we can respin the RC soon.
>
>
> Longer version:
> ===============
> When attaching a new link will always be created for global shared
> subscriptions.  This is different from before where if it is the same
> connection the link would be recovered.  But I do not think that this
> is observable by a client (other than through dumpLinkRegistry).
>
> When the subscription is ended (unsubscribe) then we need to perform a
> null-source lookup in the registry.  In our previous work we already
> changed this from vanilla AMQP 1.0 to allow the lookup to include a
> search for different remote container-ids.  I did not change this.
> Instead when detaching we ensure that there remains at least one link
> in the registry representing the subscription in order to facilitate
> the unsubscribe.
>
> The change is relatively small and only touches SendingLinkEndpoint if
> you disregard the changes necessary to undo the former code to
> disallow global shared subscriptions.
>
> The previous context variable
>   qpid.feature.disabled:globalSharedDurableSubscription
> has been removed in favour of a new one
>   qpid.jms.discardGlobalSharedSubscriptionLinksOnDetach
> Obviously, the semantics have changed.
>
> TL;DR:
> ======
> The attached patch should allow global shared subscriptions,
> not leak links in the registry, and give us a certain amount of
> confidence that it does not affect other areas of the code.
>
> Kind regards,
> Lorenz
>
> [1] https://issues.apache.org/jira/browse/QPID-7998
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
> For additional commands, e-mail: users-help@qpid.apache.org
>

Hi Lorenz,

This sounds/looks good to me overall, though I wonder around whether
the 'more than 1 left' checking + subsequent calls are racey (e.g
across different connections with subscribers detaching at the same
time) and if so about the consequences.

Robbie

---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@qpid.apache.org
For additional commands, e-mail: users-help@qpid.apache.org


Mime
View raw message