[
https://issues.apache.org/jira/browse/QPID-3269?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13061442#comment-13061442
]
[email protected] commented on QPID-3269:
-----------------------------------------------------
bq. On 2011-07-07 16:28:47, Robbie Gemmell wrote:
bq. > This seems a bit horrible, deliberately leaving a session open and
unused. Were there really no alternative options?
bq. >
bq. > 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.
bq. >
bq. > 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?
bq.
bq. Gordon Sim wrote:
bq. 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!
bq.
bq. 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).
bq.
bq. 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.
bq.
bq. However, I'd genuinely be interested in alternative approaches if you
have any suggestions.
As for the approach - I am quite keen on using something that works with any
0-10 complaint broker, rather than implementing anything qpid specific. This
workaround was needed to ensure that the modification was only client side. The
other alternative that Gordon mentioned requires modification to both brokers
and has the disadvantage of not working with non qpid brokers.
I also didn't quite understand about how this would cause more sessions to be
created than allowed? The session created here is using the same method as any
other session would. So it contains all the necessary checks and balances,
including getting a channelID, registering with the connection, recreating
after failover, closed when the connection is closed ..etc
As for throwing the JMSException instead of using a boolean.
The createSession method throws a JMSException, so either I need to handle it
or pass it up the stack. I was also hoping that I don't have to handle the
JMSException at all in the verifyClientID method in AMQConnection. But
unfortunately we don't throw a JMSException in the AMQConnection ctor, rather
an AMQException.
So it's the same story - either I handle it at the delegate level or at the
AMQConnection level. Both looks ugly.
I think a better solution is to have the ctor in AMQConnection throw a
JMSException. Our client should only throw JMSException to the application, not
any Qpid specific exceptions.
- rajith
-----------------------------------------------------------
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:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/1027/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-07-07 02:17:42)
bq.
bq.
bq. Review request for qpid, Gordon Sim and Robbie Gemmell.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. 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.
bq.
bq. In summary the following changes were made in order to support the above,
bq. 1. A verifyClientID method was added to the connection delegates,
bq. 2. AMQSession_0_10.java was modified to allow a name to be specified for
the underlying AMQP session.
bq. 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).
bq. 4. Setter/Getter in Session.java to store/retrieve the SessionDetachCode
and ConnectionDelegate to set the detach code.
bq. 5. SessionDelegate to notify Session object when attached/dettached/closed
is invoked.
bq.
bq.
bq. This addresses bug QPID-3269.
bq. https://issues.apache.org/jira/browse/QPID-3269
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnection.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_0_10.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionDelegate_8_0.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession_0_10.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/client/src/main/java/org/apache/qpid/client/XASessionImpl.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/ConnectionDelegate.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/Session.java
1143628
bq.
http://svn.apache.org/repos/asf/qpid/trunk/qpid/java/common/src/main/java/org/apache/qpid/transport/SessionDelegate.java
1143628
bq.
bq. Diff: https://reviews.apache.org/r/1027/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. A patch containing a test will be attached to the JIRA shortly.
bq.
bq.
bq. Thanks,
bq.
bq. rajith
bq.
bq.
> Concurrently executing connections are allowed to use the same client ID
> -------------------------------------------------------------------------
>
> Key: QPID-3269
> URL: https://issues.apache.org/jira/browse/QPID-3269
> Project: Qpid
> Issue Type: Bug
> Components: Java Client
> Affects Versions: 0.6, 0.8, 0.10
> Reporter: Rajith Attapattu
> Assignee: Rajith Attapattu
> Priority: Minor
> Fix For: 0.11
>
>
> Section 4.3.2 of the JMS spec says that the JMS provider
> should prevent concurrently executing clients from using the same client id.
> Qpid JMS client allows two connections to be created using the same client id.
> How reproducible:
> Always
> Steps to Reproduce:
> 1. Create two connections with the same client ID.
> 2. Observe that there are no exceptions being thrown.
> Actual results:
> No exception being thrown.
> Expected results:
> An exception should be thrown.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project: http://qpid.apache.org
Use/Interact: mailto:[email protected]
