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 2022/11/07 09:49:04 UTC
[GitHub] [activemq-artemis] AntonRoskvist opened a new pull request, #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
AntonRoskvist opened a new pull request, #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281
I have observed this happening with far fewer messages and multiple consumers attached to the same queue, but have been unable to write a consistent test for it. It also happens with AUTO_ACKNOWLEDGE instead of transacted sessions but this was easier to implement in a test.
I'm not really happy with the runtime of the test so if anyone has suggestions on how to make it run faster just let me know.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307225329
This also happens with Session.AUTO_ACKNOWLEDGE (which is the case where I first noticed this) though. The reason I use this many messages and a transaction is because that's the way I could make the test work reliably.
It can also happen with far fewer messages rolled back (but still many) and with multiple consumers on the same queue.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307172637
@AntonRoskvist I just saw your test. You're not really fixing it.. you're just moving the "issue" in a way it would only fail in the XA scenario.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307619110
Let's keep it open for now ?
I have been coming back and forth the two branches for testing. And it makes me easier if it's open.
I can close it when I'm done.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] jbertram commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
jbertram commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1305913524
I don't think this fix is valid as it's changing the semantics of the transaction rollback when a session fails. Previously _any_ pending transaction was rolled back whether or not it has a xid. Now you're only rolling back transactions with a xid which means in your test the transactions aren't being rolled back at all. It's perfectly valid for a transaction not to have a xid and such transactions should be rolled back when a session fails.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1308349850
Very interesting!
I think the issues comes down to the transaction getting handled as two consecutive rollbacks for some reason... logging out refs.size() in QueueImpl shows that the first run rolls back some 90% of all messages and this is really fast... then the rest of the messages gets rolled back and I believe these arrive out of order such that the list has to be scanned for each message to find it's proper place...
And yes, I believe the CriticalAnalyzer is absolutely right to stop the broker. In fact I tried to set it to LOG and the broker was never able to recover properly... or at least not for some 30 minutes which is how much time I gave it before manually restarting.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307329784
@AntonRoskvist https://github.com/apache/activemq-artemis/pull/4283
I'm still improving the test (to make sure it fails without the fix, as the test is always passing now);
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1308820712
@AntonRoskvist it is about the two consecutive rollbacks. One will add a bunch of data. The second will call the addSorted for a huge list, scanning all the lists for every rollback item.
If you actually rolled back the consumer's pending first, the adds would be in order as the addSorted checks for first and last.
I'm adding an optimization to keep the last added position and also check against the last added, and I will as well have the adds done outside of the locking.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307359141
@clebertsuconic when this happened with Session.AUTO_ACKNOWLEDGE there where 10 consumers running with 5 or 10MB as limit for consumerWindowsSize. The consuming application ran into issues with it's own resources so processing slowed down significantly (so all consumer buffers filled up), then the application either crashed or where killed, not sure which, but the result on the broker side was this.
I suppose if the test passes it might be load related, I only tried this out on a... "not super performant laptop". Could you possibly try increasing the messageCount?
I also just realized... in my test case there is only a single transacted session, why is the rollback issued twice (with one very large and then a quite small batch)? I suppose that might put high pressure on the "org.apache.activemq.artemis.utils.collections.LinkedListImpl.addSorted()" if the large batch arrives in order and then the smaller batch does not, such that the entire list has to be checked for every message in batch 2?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic closed pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic closed pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
URL: https://github.com/apache/activemq-artemis/pull/4281
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1308104682
hmmm... I think it's just a deadlock.. the rollback is holding a lock on queue, while the rollback on the large tx is getting a new lock.
I mean just because I thought there was a bug in the LinkedList. Leave it with me for another day please?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307223610
That second rollback is for the case of a non transactional session. We still use a transaction to return the messages but we would be doing something else underneath 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307215574
@clebertsuconic @jbertram Sorry, I see that I was very unclear. The rollback happens here: org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl#571
It's part of the consumer.close(failed)-call made from the "ServerSessionImpl" just after the code change I made here. Looking at the PR I mentioned above I can clearly see I was wrong though, a rollback was made previously as well.
I arrived at this from the stacktrace the CriticalAnalyzer logs when this issue occurs:
https://paste.mozilla.org/HiXUh8yg/raw
Then I set up this test and set things up so I printed a thread dump in:
org.apache.activemq.artemis.core.server.impl.QueueImpl.addSorted(QueueImpl.java:1169)
There I could see that two calls where made during client failure, the later of which seems to lock something up. Thread dumps seen here:
https://paste.mozilla.org/9nipgJbL/raw
Notice the divergence at: org.apache.activemq.artemis.core.server.impl.ServerSessionImpl.doClose
Both thread dumps are from the same thread, the first call inserted maybe 90% of all references and the second the rest. The second call is the one that locks.
This change gets rid of the first call (txToRollback.tryRollback()) but keeps the one in org.apache.activemq.artemis.core.server.impl.ServerConsumerImpl (tx.rollback())
I am very open to the possibility of me being wrong here though
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1309235109
Excellent, thanks @clebertsuconic !
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1309074752
I got all the way to the bottom of the issue. #4283 will definitely fix the issue here
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307217479
Well, I am not sure the use case is valid though... a huge Transaction like this is certainly bound to a failure.
I'm looking if there is any evidence of a dead lock here though before I say anything.
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307564496
@clebertsuconic Sorry, I missed your link there, that's looking very good I think! Should I just close 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307184187
So, if you did the same test using a XA TX, you wouldn't have fixed it, right?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1309074324
@AntonRoskvist I am closing this PR as this is being replaced by #4283
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] AntonRoskvist commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
AntonRoskvist commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1305958085
They are getting rolled back, that's part of the close() call issued just a little bit later. (tx.rollback() on line #571 in my change)
The test also makes an assertion on messageCount right at the end.
The previous (much older) behavior was that only XA transactions was handled here as well:
https://github.com/apache/activemq-artemis/commit/bafefdc8ece23801ac8fb7f5879faa9c3107ae5f
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307031042
@AntonRoskvist I am confused.. where the rollback is happening for non XA transactions?
line 571 is a a call to server.callBrokerConsumerPlugins(plugin -> plugin.afterCreateConsumer(consumer));
I am also having the impression that a rollback is not happening. I'm not doubting you.. I just can't see.. because perhaps there's a better change for 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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1307870524
@AntonRoskvist the issue is not on the transactions itself. Your rollback ordering here is uncovering some bug on LinkedListImpl::addSorted. For some reason this will only happen if you used the Core API. Some sort of duplicate adds is probably causing it, because if I simply used a regular ConnectionFActory it worked fine.
if you go to LInkedListImpl::addSorted, there is something in a loop after the rollback.
The CriticalAnalyzer actually killed your broker correctly on this case... as the logic was lost.
I will need to work on it tomorrow... but if you could also take a look on that?
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [activemq-artemis] clebertsuconic commented on pull request #4281: ARTEMIS-4084 Rollbacking massive amounts of messages might crash broker
Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on PR #4281:
URL: https://github.com/apache/activemq-artemis/pull/4281#issuecomment-1308108324
not a dead lock... it's just the way this rollback happens it is taking a long time to process. relaxing the locks is the way to go here.
I will see if I can do some optimization.
will be fixed tomorrow :)
it will be after 2.27.0, but we should have a 2.28.0 soon..
--
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.
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org