Respect max_inlight message count in reconnect #698
Conversation
This file contains 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
Juha-Ylikoski-Treon
changed the title
Previously if the message queue filled up for some reason like bad connection and the client disconnected (due to e.g. bad connection), it would send all of it's messages in _out_messages-queue and not respect _max_inflight_messages parameter. With some brokers when we had 1000s of messages in queue and all being sent with qos 1 without any regard to the _max_inflight_messages, the broker would disconnect us or the connection would again break down. This patch makes sure _max_inflight_messages is respected when reconnect happens for any reason. I believe this also fixes eclipse-paho#492 Signed-off-by: Juha Ylikoski <juha.ylikoski@treon.fi>
Juha-Ylikoski-Treon
force-pushed
the
respect-max_inflight-on-reconnect
branch
from
eb47eb8
to
86d401a
Compare
PierreF
added a commit
that referenced
this pull request
Dec 25, 2023
This test is currently broken, PR #698 should fix/improve the situation
|
Thanks for your contribution. I've written some test in a2574e6 (pushed on master) which show the problem exist on master. It also show that there is another problem: message are published twice, once with mid=original_mid + N and once with mid=original_mid & dup = 1. This issue still exists on this PR. I haven't yet looked whether the two issues are unrelated (and then the PR should be merged as-is) or they are related and should be fixed together. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously if the message queue filled up for some reason like bad connection and the client disconnected (due to e.g. bad connection), it would send all of it's messages in _out_messages-queue and not respect _max_inflight_messages parameter. With some brokers and when we had 1000s of messages in queue and all being sent with qos 1 without any regard to the _max_inflight_messages, the broker would disconnect us or the connection would again break down.
This patch makes sure _max_inflight_messages is respected when reconnect happens for any reason.
I believe this also fixes #492
Signed-off-by: Juha Ylikoski juha.ylikoski@treon.fi