qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Robert Godfrey <rob.j.godf...@gmail.com>
Subject Re: Review Request: Concurrently executing connections are allowed to use the same client ID
Date Thu, 07 Jul 2011 17:36:11 GMT
On 7 July 2011 18:43, Gordon Sim <gsim@redhat.com> wrote:

>
>
> > On 2011-07-07 16:28:47, Robbie Gemmell wrote:
> > > This seems a bit horrible, deliberately leaving a session open and
> unused. Were there really no alternative options?
> > >
> > > On first glance I also don't imagine this works with the Java broker,
> which would admittedly be the brokers fault but has it been tested? There is
> a MaxChannels value that gets negotiated with the broker during the AMQP
> connection, and the the client currently uses it to provide feedback to
> users when they exceed the allowed number of JMS Sessions, I think this will
> probably break that and allow more Sessions to be created/attempted than
> should be.
> > >
> > > When the clientid is being verified the method declares it throws
> JMSException from the delegate, but Exception is caught by the calling
> method instead. Would a boolean return not suffice here, with exceptions
> only being thrown from the verification method due to unexpected occurences?
>
> Re "This seems a bit horrible, deliberately leaving a session open and
> unused. Were there really no alternative options?" - I'm to blame for that
> approach!
>
> I think if possible using a standard feature of the protocol is desirable.
> The only alternative I could see was some Qpid specific extension, e.g. have
> the client id in the properties on start-ok and have the broker understand
> this and reject duplicates (would need to reuse one of the limited close
> codes here though which isn't ideal either).
>
> An AMQP session is generally quite lightweight. Yes, it uses up one
> channel, but that's really all. The upside is that it should work for any
> compliant 0-10 broker.
>
> However, I'd genuinely be interested in alternative approaches if you have
> any suggestions.
>
>
Is there a way we could do this by using temporary queues and binding with
the client name to an exchange ... I thought that Exchange.Bound would tell
you if there is or isn't any queue already bound with a given binding key...
though the definition isn't particularly clear as you seem always to have to
give a queue name.  The temp queue would obviously by destroyed when the
connection dies.  not sure if this is more or less ugly than the given
approach.

Realistically I imagine that there are unlikely to be any more AMQP 0-10
brokers, with AMQP 1.0 soon to be released... and as Robbie says, I'm pretty
sure the current approach will only work with 50% of them (i.e. the C++
broker) :-)

-- Rob


>
> - Gordon
>
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1027/#review989
> -----------------------------------------------------------
>
>
> On 2011-07-07 02:17:42, rajith attapattu wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/1027/
> > -----------------------------------------------------------
> >
> > (Updated 2011-07-07 02:17:42)
> >
> >
> > Review request for qpid, Gordon Sim and Robbie Gemmell.
> >
> >
> > Summary
> > -------
> >
> > In order to verify the uniqueness of the client ID, a dummy session is
> created using client ID as it's name. This prevents any other connection
> from using same client ID as the session creation will fail. However this
> verification is switched off by default in order to preserve backwards
> compatibility. You need to use -Dqpid.verify_client_id=true switch
> verification on.
> >
> > In summary the following changes were made in order to support the above,
> > 1. A verifyClientID method was added to the connection delegates,
> > 2. AMQSession_0_10.java was modified to allow a name to be specified for
> the underlying AMQP session.
> > 3. A method was added to o.a.q.transport.Session.java to wait until the
> session state was changed from NEW to OPEN (or another state which triggers
> the error).
> > 4. Setter/Getter in Session.java to store/retrieve the SessionDetachCode
> and ConnectionDelegate to set the detach code.
> > 5. SessionDelegate to notify Session object when
> attached/dettached/closed is invoked.
> >
> >
> > This addresses bug QPID-3269.
> >     https://issues.apache.org/jira/browse/QPID-3269
> >
> >
> > Diffs
> > -----
> >
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java1143628
> >
> http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java1143628
> >
> > Diff: https://reviews.apache.org/r/1027/diff
> >
> >
> > Testing
> > -------
> >
> > A patch containing a test will be attached to the JIRA shortly.
> >
> >
> > Thanks,
> >
> > rajith
> >
> >
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message