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

Occasional invalid PINGRESP timed out (local broker) #237

Closed
MattBrittan opened this issue Feb 10, 2024 · 2 comments · Fixed by #238
Closed

Occasional invalid PINGRESP timed out (local broker) #237

MattBrittan opened this issue Feb 10, 2024 · 2 comments · Fixed by #238

Comments

@MattBrittan
Copy link
Contributor

Describe the bug

Connecting to a local (client and broker in docker containers; host is EC2 VM) broker and seeing occasional disconnections due to PINGRESP timed out.

To reproduce

Don't have a reproducer; however added info to the error (return fmt.Errorf("PINGRESP timed out (%v, %v)", lastPingSent, lastPingResponse)) and got:

ping handler error: PINGRESP timed out (2024-02-11 11:54:04.033683423 +1300 NZDT m=+1.344078227, 2024-02-11 11:54:04.033226938 +1300 NZDT m=+1.343621806)

So the lastPingSent is a fraction of a second after the lastPingResponse

Believe the issue is that the ping response is received pretty much instantly. Moving the lastPingSent = time.Now() above the packets.NewControlPacket(packets.PINGREQ).WriteTo(conn) should solve this (have this running). However this fix would also increase the possibility of timeouts occurring when the connection is highly utilised; so will have a think about better options.

Expected behaviour

Don't really expect timeouts with this setup (could happen very occasionality)

Software used:

  • Server was Mosquitto 2.0.18
  • Client version 0.20
MattBrittan added a commit to ChIoT-Tech/paho.golang that referenced this issue Feb 12, 2024
When running locally, there is a chance that `NewControlPacket(packets.PINGREQ).WriteTo(conn)`
may return after the ping response has been handled. This led to invalid timeouts.

Closes eclipse#237
@thejan2009
Copy link
Contributor

Hey, is there any chance this fix could land in a patch release?

@MattBrittan
Copy link
Contributor Author

Yep - did not know anyone else was seeing 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

Successfully merging a pull request may close this issue.

2 participants