moscajs / mosca

MQTT broker as a module
mosca.io
3.2k stars 509 forks source link

Prevent FIN_WAIT connections by using stream.destorySoon() #524

Closed eladnava closed 8 years ago

eladnava commented 8 years ago

As per #306.

mcollina commented 8 years ago

I'm thinking we can have a three way case: destroySoon, destroy and end. In this order.

What do you think?

eladnava commented 8 years ago

@mcollina Done, however I wasn't able to run the tests completely as I don't have Redis installed.

mcollina commented 8 years ago

@behard what do you think?

eladnava commented 8 years ago

I think you meant @behrad =)

mcollina commented 8 years ago

Oh yes :D.

behrad commented 8 years ago

So it should reduce FIN_WAITs :) If destroySoon is preferable, how will that affect on the mobile/public facing client end?

eladnava commented 8 years ago

@behrad When connected clients passively disconnect from the internet, they will eventually be kicked due to Mosca's keepalive timeout mechanism, which calls socket.end() if the client hasn't sent an MQTT keep alive packet within the client-defined time limit.

In its current implementation, calling socket.end() on those connections will have them stuck (if not indefinitely, then for a long time) in FIN_WAIT states because the TCP stack will attempt to let the client know it's closing down the connection. But since the client is no longer reachable then the server will never be able to deliver the FIN/ACK packet to the client, nor will it receive a FIN/ACK or an ACK from the client in response to the FIN/ACK.

ssldig09

Does that answer your question?

behrad commented 8 years ago

great explanation @eladnava 👍 I can test to see how this patch will change the real world scenario :)

eladnava commented 8 years ago

@behrad excellent, please let us know if it's OK to merge :)

mcollina commented 8 years ago

Ping @behrad? I'll like to merge this and release next week!

behrad commented 8 years ago

Sorry guys, so busy these days, it takes me some time to test this, I'l let you even after @mcollina releases this :)

eladnava commented 8 years ago

@mcollina Has this been released yet? I'd love to use it in production. Thanks! 😄

mcollina commented 8 years ago

Sorry, released now as v2.1.0.

eladnava commented 8 years ago

Excellent, thank you! 👍