apinf / mqtt-bridge

0 stars 0 forks source link

Disconnection event from emqttc is not reliably delivered #1

Closed phanimahesh closed 6 years ago

phanimahesh commented 6 years ago

From the logs, looks like the disconnection event {mqttc, ClientPid, disconnected} event is not always delivered properly. Investigate.

https://github.com/apinf/mqtt-bridge/blob/9f56cb0cf7181c30dbb2a6200a83f624fed09eac/src/mqtt_bridge.erl#L61-L72

'EXIT' messages are very reliable, maybe switch to reconnecting on exit and log on disconnect?

phanimahesh commented 6 years ago

@sa1 Can you make sure I didn't do anything stupid in 8cecca5, then merge and test?

sa1 commented 6 years ago

I don't see anything stupid, and things worked as expected on testing, however, I was unable to find instance where disconnection events were not delivered. Logging disconnections will definitely help.

I think it was the "Connection refused" events {shutdown, econnrefused} after which we failed to restart the client. Trapping exits needs to be done carefully. This commit definitely fixes this issue, though we ought to leave this to emqttc.

Merged.

phanimahesh commented 6 years ago

That's what initially made me write it the old way. It's a relatively rare event when disconnection doesn't trigger before exit. However even rare events happen frequently given enough time.

Also, starting a client before the old one definitely exited can cause a mutual ass-kicking contest since both clients use same id. This way we avoid that too.

This also frees us up to use reconnect option of emqttc.

On 21-Dec-2017 04:32, "Saurabh Kumar" notifications@github.com wrote:

I don't see anything stupid, and things worked as expected on testing, however, I was unable to find a EXIT event without a disconnected event. Logging disconnections will definitely help.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/apinf/mqtt-bridge/issues/1#issuecomment-353208307, or mute the thread https://github.com/notifications/unsubscribe-auth/ABnL8MGH7uYWuQFmzndujM3RPF6ck6R0ks5tCZIMgaJpZM4RDcum .