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 2021/03/11 10:36:57 UTC

[GitHub] [activemq-artemis] meierhofer08 opened a new pull request #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   During session reattachment, also set the new connection on the
   "session" member of the ServerSessionPacketHandler. Until now,
   the connected ServerSessionImpl instance still referenced the old
   connection although it had already been transferred on the new connection.


----------------------------------------------------------------
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] meierhofer08 commented on pull request #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   Ok perfect, thank you for merging and ensuring that there is a test for the change!


----------------------------------------------------------------
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 #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   Could you possibly add a test to validate this fix and mitigate any regressions in the future?


----------------------------------------------------------------
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] meierhofer08 commented on pull request #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   That would be good of course. The only problem is that I'm not familiar with the testing structure of Artemis/how to set up mocks for testing etc. It would help if you could point me into a direction for where to place the test and how to achieve what I want to test.
   
   The test would need to:
   - Set up an Artemis core client connection to a broker with a confirmationWindowSize != -1
   - Simulate connection loss to broker, the client has to detect the connection failure ("Connection failure to ... has been detected")
   - After less than the connectionTTL (default 60 seconds), allow connection to broker again -> client fails over
   - Client tries to reattach all the sessions, server transfers the sessions
   - Afterwards, check if the remotingConnection is set correctly (has the new connectionID) on every object connected to the session
   
   It could even be simplified to a "component test" only on the broker where you have a session with an "old" connection, and then do a "ServerSessionPacketHandler.transferConnection" suppyling a new connection. Afterwards, check if the all the objects related to the sessionHandler (itself, the channel, the serverSession) have the new connection assigned.
   
   If you provide me with enough details, I might be able to write the test. Otherwise, it would maybe be easier if you write the test as you know the code in more detail.


----------------------------------------------------------------
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 #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   Like most functionality in the broker there was an existing test that was simple to modify so I added a test and merged this PR. Thanks for the contribution, @meierhofer08!


----------------------------------------------------------------
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] asfgit closed pull request #3486: ARTEMIS-3174: Set new connection on ServerSession during reattachment

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


   


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