On 7 July 2011 18:43, Gordon Sim <[email protected]> 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 > > > > > >
