Hook up the keepAlive after a successful connect

[marcbachmann]

I haven't tested it yet as I'm running on macos and there's no way to read out the socket options, but this should fix #1339

Changelog

• 🐛 Call socket.setKeepAlive(true, ms) after a successful connect as workaround for a node issue where it won't work when it's called before a successful connect

[marcbachmann]

Alternatively we could use socket.connect({keepAlive: true, noDelay: true}[, listener]): https://nodejs.org/api/net.html#socketconnectoptions-connectlistener
That might be cleaner, but at the moment net.createConnection is in use at several places.

[luin]

Is there a link explains why it didn't work?

[marcbachmann]

nodejs/node#31663

[marcbachmann]

Passing connection options during createConnection is more straight forward. I've pushed the changes.

[luin]

Is this ready for review? I think we also need to change SentinelConnector?

[marcbachmann]

The noDelay and keepAlive properties on the connect options only got added in node 17.7.0. So I'll need to revert that change back to a manual call after connect 😢

[marcbachmann]

This is now ready. I've decided to move it back into the redis constructor to keep it in one place with the reconnect handling.
So it should work now for sentinel and regular use. I also found one small issue with sentinel where the failover event listener wasn't set up properly. That's in the second commit now.

[luin]

LGTM! Awesome 👍

[github-actions]

🎉 This PR is included in version 5.0.3 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀