qpid-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gordon Sim" <g...@redhat.com>
Subject Re: Review Request: Concurrently executing connections are allowed to use the same client ID
Date Thu, 07 Jul 2011 16:43:12 GMT


> 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.


- 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.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
1143628 
>   http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java
1143628 
> 
> 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