Skip to content
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

Resume - Simultaneous Publishes #520

Closed
MattBrittan opened this issue Jul 19, 2021 · 2 comments
Closed

Resume - Simultaneous Publishes #520

MattBrittan opened this issue Jul 19, 2021 · 2 comments

Comments

@MattBrittan
Copy link
Contributor

When the connection is established to the broker and there are messages queued the library sends them all immediately upon connection. When there are a significant number of queued messages and a cellular link is in use this appears to saturate the link resulting in the connection dropping. In addition this can result in significant numbers of out-of-order packets (not that order is guaranteed when resuming from a store).

Having performed some tests it appears that limiting the number messages in flight can significantly reduce these issues. This appears particularly bad when code is running on low power routers; I run a fair number of Teltonika rut955's and resuming on these after an outage is particularly slow (but resolved when in flight messages reduced).

As such I'll raise a PR to add a new option SetMaxResumePubInFlight that will ensure that only the resume function will only send the specified number of messages simultaneously (it will wait for the relevant ACK's before sending the next one).

Note: It would be nice to have a global MaxInFlight option but I don't currently have time to implement that (it would be a much larger change).

MattBrittan added a commit to ChIoT-Tech/paho.mqtt.golang that referenced this issue Jul 19, 2021
…stored publish packets when connection resumes.

Ref eclipse#520
@MattBrittan
Copy link
Contributor Author

Change merged - will leave this open for a while in case any issues arise (if the option is not set this change should have no impact on usage).

@MattBrittan
Copy link
Contributor Author

Closing this as I've not had any issues in production (and this change has resolved the issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant