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

decrease keepalive sensitivity to reduce disconnections #839

Closed
wants to merge 1 commit into from

Conversation

pb66
Copy link
Collaborator

@pb66 pb66 commented May 13, 2018

See https://community.openenergymonitor.org/t/a-question-over-whether-emoncms-is-compatible-with-mqtt/7180/12?u=pb66

I suspect setting a keepalive of "5" is actually resulting in MORE disconnections than a more relaxed setting of "60". But it may be better to go for "180" or more to accomadate setups with just a couple of emonTH's.

PLEASE DON'T MERGE WITHOUT DISCUSSION AND CONSIDERATION OF THE CLIENT ID PR TOO!

@glynhudson
Copy link
Member

This also sounds great. Shall I merge into master to open up the testing pool? All (or the majority) of emonPi/emonBase/emonSD users are running Stable. I can't see it causing any adverse issues

@borpin
Copy link
Contributor

borpin commented May 17, 2018

Is working for me with the keepalive removed completely so it falls back to the default. I should say though, I'm using #822 as well to catch any disconnections/reconnections.

@pb66
Copy link
Collaborator Author

pb66 commented May 17, 2018

It will have an adverse effect on anyone using non-timestamped mqtt which is 99.99% of users (Brian uses timestamped MQTT). Can we keep the discussion in the forum, in one place please? I could easily merge this if it was ready, I only created a PR to try and get some dialog going, that why I said "PLEASE DON'T MERGE WITHOUT DISCUSSION AND CONSIDERATION OF THE CLIENT ID PR TOO!".

[edit] The issue I was originally concerned about turns out to NOT be an issue. the pingreqs etc are working so the 5s keepalive is NOT causing a disconnection/reconnection, it is just causing excessive MQTT control packets, the extra sensitive keepalive setting is probably useful whilst we have connection issues, setting a keepalive of 60s could result in no data for 60s, the (depending on whether the clientid is used) it will post the 1min old data as "now" or the data will have been lost in the dis/re-connection.

@glynhudson glynhudson requested a review from TrystanLea June 14, 2018 13:12
@glynhudson glynhudson closed this Jun 14, 2018
@TrystanLea TrystanLea deleted the pb66-patch-2 branch February 28, 2019 15:44
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

Successfully merging this pull request may close these issues.

3 participants