ARTEMIS-6217 mitigate shutdown stacktraces for MQTT - #6653
Open
jbertram wants to merge 1 commit into
Open
Conversation
| @@ -50,6 +57,8 @@ public int sendMessage(MessageReference ref, | |||
| int deliveryCount) { | |||
| try { | |||
| session.getMqttPublishManager().publishToClient(ref.getMessage().toCore(), consumer); | |||
Contributor
There was a problem hiding this comment.
the right thing would be to make sure these are flushed before. the stop on the MQTPPProtocolManager flushing everything would be a better fix. (the stop is one place it could / should block until things are done).
Contributor
Author
There was a problem hiding this comment.
I'm exploring this idea... 🤔
Contributor
Author
There was a problem hiding this comment.
I looked into what you suggested here and I have a few thoughts:
- There is no
stop(or equivalent) method for protocol managers. However, it's possible for the protocol manager to register an activation callback and leverage, e.g.org.apache.activemq.artemis.core.server.impl.CleaningActivateCallback#deActivate. - In order to wait for MQTT tasks to finish I now have to track them. This means adding logic to the hot path for all MQTT packet handling rather than just dealing with these exceptions. It is arguably more "correct" to handle shutdown this way, but there is a cost, and I'm not sure that cost is worth paying.
I'd love your thoughts.
When the broker is processing MQTT packets (especially those related to QoS 2 message flows) a shutdown can cause the broker to log spurious messages, including several different stack-traces that are effectively harmless, but can alarm users. The broker should detect these and avoid logging them. Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the broker is processing MQTT packets (especially those related to QoS 2 message flows) a shutdown can cause the broker to log several different stack-traces that are effectively harmless, but can alarm users. The broker should detect these and avoid logging them.