You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/10/14 02:59:33 UTC

[GitHub] [activemq-artemis] hezhou66 opened a new pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

hezhou66 opened a new pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302


   ARTEMIS-2948


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] hezhou66 closed pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
hezhou66 closed pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-715619986


   @hezhou66 you know that if you had added a test on the ./testsuite/compatibility-tests this would been merged already, right?
   
   
   I was planning to it myself, however I'm swamped with my own PR as you can probably see.
   
   
   Please consider adding the test.. or try to recruit someone else to do it.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] hezhou66 commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
hezhou66 commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-708845681


   > What are you trying to fix here ?
   > 
   > There are no tests. Take a look at the compatibility tests where I play with many versions as part of class loading.
   
   Thanks for reply . The tests are complicated and difficult at one project.  The process trace detail as follow ：
   
   The client version is 2.4.x whitch has not SessionSendMessage_V2, sends SessionSendMessage packet to the server
   
   The server version is 2.12.x , use ServerPacketDecoder.decodeSessionPacket method to decode message packet, but the method decode it to SessionSendMessage or SessionSendMessage_V2 depending on the connection version whitch has been set to server version(2.12.x) after failover reattach.
   
   So the SessionSendMessage was decoded as SessionSendMessage_V2, losing the last Long correlationID, causing CoreMessage.decode error.
   
   ![ServerPacketDecoder decodeSessionPacket](https://user-images.githubusercontent.com/4961665/96067600-b9e76900-0ecc-11eb-9a30-7f76f7835de7.jpg)
   ![ActiveMQPacketHandler handleCreateSession](https://user-images.githubusercontent.com/4961665/96067634-cb307580-0ecc-11eb-8d5b-5ba1caef16e1.jpg)
   ![ActiveMQClientProtocolManager newSessionContext](https://user-images.githubusercontent.com/4961665/96067645-cff52980-0ecc-11eb-851e-522e1161cdba.jpg)
   ![ActiveMQSessionContext newCreateSession](https://user-images.githubusercontent.com/4961665/96067654-d2f01a00-0ecc-11eb-8ac6-6b7255164d4f.jpg)
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] hezhou66 commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
hezhou66 commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-720852135


   I'm sorry to get back so late.    I found  it was my own mistake.    I used standalone -- "live-only" policy, and I rewrote the client code by removing toplogy subscriber whitch I thought not useless in standalone model. But in witch  I  found the  version negotiation. Although the version number sored in the server  is not just the compatible versions, it is functional.
   
   Once again, I'd like to apologize to you all！
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] jbertram commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
jbertram commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-716690552


   @hezhou66, I'm looking to create a test-case for this to validate your fix and prevent potential regressions in the future. Can you elaborate on the full use-case? What is the minimum steps required to reproduce this?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-720867428


   @hezhou66 no problem.. I'm glad it's ok...
   
   If you find an issue around this or anywhere else.. a test case would help a lot... 
   
   We can sometimes do with descriptions and do our best. but a test case always improves speed.
   
   I'm glad this was not an issue. (as I understand).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [activemq-artemis] clebertsuconic commented on pull request #3302: ARTEMIS-2948 & ARTEMIS-1187 use negotiated compatible version

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on pull request #3302:
URL: https://github.com/apache/activemq-artemis/pull/3302#issuecomment-708141386


   What are you trying to fix here ?
   
   
   There are no tests.   Take a look at the compatibility tests where I play with many versions as part of class loading. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org