[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119549290
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

Thanks, I'm glad for feedback, I might have also be wrong...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/activemq-artemis/pull/1298


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119394970
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

Ohhh.. I see.. you're right!


merging it.. sorry for the hassle.. just wanted to make sure what was going 
on.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

Github user TomasHofman commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119296506
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

The ```newCreateSession()``` is only called in ```recreateSession()``` 
method, so my reasoning was that the message will be sent to the same server 
that the client was previously connected to (is that correct?), so the 
serverVersion that was previously established can be reused.

The alternative could be to start a cycle of CreateSessionMessages with the 
highest know version and decrease the version if 
```ActiveMQIncompatibleClientServerException``` is returned? I think this is 
how establishing new connection works.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[clebertsuconic]

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1298#discussion_r119236680
  
--- Diff: 
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ActiveMQSessionContext.java
 ---
@@ -686,7 +686,7 @@ protected CreateSessionMessage newCreateSession(String 
username,
boolean autoCommitSends,
boolean autoCommitAcks,
boolean preAcknowledge) 
{
-  return new CreateSessionMessage(name, sessionChannel.getID(), 
VersionLoader.getVersion().getIncrementingVersion(), username, password, 
minLargeMessageSize, xa, autoCommitSends, autoCommitAcks, preAcknowledge, 
confirmationWindow, null);
+  return new CreateSessionMessage(name, sessionChannel.getID(), 
serverVersion, username, password, minLargeMessageSize, xa, autoCommitSends, 
autoCommitAcks, preAcknowledge, confirmationWindow, null);
--- End diff --

I'm not sure about this... the purpose is the client to tell the server 
which version is being used... it looks wrong to me.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] activemq-artemis pull request #1298: ARTEMIS-1187 Incompatible version when ...

[TomasHofman]

GitHub user TomasHofman opened a pull request:

https://github.com/apache/activemq-artemis/pull/1298

ARTEMIS-1187 Incompatible version when recreating a session with olde…

…r server

https://issues.apache.org/jira/browse/ARTEMIS-1187
https://issues.jboss.org/browse/JBEAP-9522

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/TomasHofman/activemq-artemis JBEAP-9522

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/activemq-artemis/pull/1298.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

This closes #1298


commit c3871a183f48b6cccaa3a65e7c14b2b64e16b39d
Author: Tomas Hofman 
Date:   2017-05-29T10:27:09Z

ARTEMIS-1187 Incompatible version when recreating a session with older 
server




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---