Replaced ping-based healthcheck with net.createConnection

mutantcornholio commented 5 years ago

Older ping-based healthcheck had a bunch of problems:

Doesn't work in ipv6-only environment (because of calling ping, not ping6).
Doesn't work if ICMP is blocked, but TCP is not
Doesn't work as expected if TCP on this exact port is firewalled out.
Uses a lot of CPU on process spawning, may cause DoS in clusters with large number of worker processes.

Healthcheck with net.createConnection seems to be a much simpler and more robust alternative

Two test were needed changing:

One was implicitly tied to ping interval; ECONNREFUSED was thrown faster, I've tuned reconnect timeout in test
Another one actually relied upon one of aforementioned problems: setting up server at 127.0.0.1:1234 would mean server is available to ping, but not to TCP connection. In that test, I've used net.listen to actually listen for connections on socket

mutantcornholio commented 5 years ago

Test are faililng on nodejs 4/5 because mocha@^4 uses object destructuring, which isn't supported by those versions of node.

Downgrading to mocha@^3 helps

mutantcornholio commented 5 years ago

@3rd-Eden what do you think?

mutantcornholio commented 5 years ago

@mapleeit, @kevin-greene-ck, looks like you are maintainers now?

mutantcornholio commented 5 years ago

Anyone? @3rd-Eden @mapleeit @kevin-greene-ck ?

aakarsh commented 4 years ago

@mutantcornholio @3rd-Eden Any one working to integrate this, we are in environment without ping command, this will help us.

mutantcornholio commented 4 years ago

This PR is the only difference in code now

3rd-Eden / memcached