qpid-users mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lorenz Quack <quack.lor...@gmail.com>
Subject Re: Accumulation of Links from global shared durable subscriptions (JMS 2.0)
Date Tue, 07 Nov 2017 10:26:44 GMT
On Tue, 2017-11-07 at 09:51 +0000, Robbie Gemmell wrote:
> 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

Hi Robbie,

Well spotted. It is known to be racey.
In the worst case the last two links would unsubscribe concurrently and
both be removed from the link registry.  Upon unsubscribe the link would
not be found in the registry and the attach would be rejected with
"not-found". The subscription queue would remain on the broker.
There are other races connected to shared subscriptions / link registry.
That does not make it better but we really want to get the release out
so we created QPID-7996 to address these issues in 7.0.1. 

We are going to back port the change an spin a RC2 today.

Kind regards,
Lorenz


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


Mime
View raw message