-
Notifications
You must be signed in to change notification settings - Fork 317
Сonvert ActiveMQConnectionFactory to SingleConnectionFactory in jms-sender-applicationContext.xml #1549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ender-applicationContext.xml Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
|
Thanks for the PR!
I don't think this is accurate. Temporary queues are created for exactly that purpose. Sender creates a temporary queue in its session, sends a message with a I haven't been able to reproduce the problem reliably so I am not sure what to do for this yet. |
|
Thanks for digging into this! You’re absolutely right about how TemporaryQueue works. What I found in our tests is that JmsMessageSender and JmsTemplate were each opening their own JMS Connection, so the queue created on Connection A could be closed (and deleted) before Connection B tried to reply. By switching both sides to use a single shared ConnectionFactory (via SingleConnectionFactory), they both reuse the same Connection/Session, so the temp queue stays alive until the reply is sent and the flakiness goes away, I think. |
Yes but that's a problem in the receiver end of things.
You are probably correct but that's hiding the underlying problem. I'd rather make the test more robust rather than hiding the fact the receiver is closing the conection when it shouldn't have. Thanks for the PR and the convo, in any case! |
|
As much as I dislike it, I think there's not much else we can do given the timing of the transactions. |
This commit updates the JMS integration tests to use a SingleConnectionFactory rather than a regular ActiveMQConnectionFactory. Given that the tests exercises both the sender and the receiver in the same test, we can guarantee reliably the boundary of the transaction. See gh-1549 Signed-off-by: Andrey Litvitski <andrey1010102008@gmail.com>
The configuration had moved to Java config and the XML file wasn't remove. This is done as part of this commit. See gh-1549
|
Actually this change makes it worse from what I see on CI. Given it hasn't resolved the problem I am going to revert it. |
The change didn't fix the underlying issue. Looking at CI it even seems to have made it worse. See gh-1549
I've been thinking about this for a very long time to be honest, and have come to one conclusion, please correct me if I'm drastically wrong.
However, the point is that I believe that when we test sending from TemporaryQueue that is created when each connection is created, it can't be synchronized. The way I understand it is this:
And then it turns out all tests where we test TemporaryQueue, they should be synchronized. I have made all ChannelFactory in the tests to be SingleConnectionFactory, however I guess this is necessary for tests where we use TemporaryQueue, but it would be easier to do it for all tests.
If I am misunderstanding something, please correct it
Fix: #1539