net: activate setNoDelay by default

[voxpelli]

Fixes #34185

This is kind of a draft and is eg. lacking tests as I don't know how to test this (partly because it interacts deeply with the OS) and the fact that the current setup isn't tested either, so its an extra uphill in creating tests for this tiny change of default.

So, remaining:

• Implement change
• Test change
• Document change

I'm opening a PR anyway, so I don't block #34185 and maybe can get some help on how to proceed.

[mcollina]

This is kind of a draft and is eg. lacking tests as I don't know how to test this (partly because it interacts deeply with the OS) and the fact that the current setup isn't tested either, so its an extra uphill in creating tests for this tiny change of
default.

There is a failed test already that's related to this change. I think we can just update the test and we are good.

[mcollina]

I flagged this as minor but baking-for-lts. We might decide if we want to backport it or not later.

cc @nodejs/tsc

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/40849/

[voxpelli]

There is a failed test already that's related to this change. I think we can just update the test and we are good.

I have now adapted the tests to take into account that the new default is true and thus in a way inverted the expectations on call counts and such.

[addaleax]

This definitely needs documentation, both in the docs themselves and in the changes: entries list(s).

I would definitely recommend not backporting this to LTS versions.

[voxpelli]

@addaleax 👍 Any pointers on where/how I can document this? Sorry, first PR to Node core for me

[lpinca]

Is there a reason for changing a default instead of disabling the Nagle's algorithm before a request is made? Wouldn't it be better to call socket.setNoDelay() before emitting the 'socket' event?

node/lib/_http_client.js

Line 757 in 9cd3089

req.emit('socket', socket);

By doing this, the change would only affect the HTTP/S case and not all created sockets that could be used for something else.

[voxpelli]

Is there a reason for changing a default instead of disabling the Nagle's algorithm before a request is made? Wouldn't it be better to call socket.setNoDelay() before emitting the 'socket' event?

To make it still be configurable?

Also: Is there any case where we want to have this enabled by default?

[lpinca]

I think it's enabled by default on Linux? My point is, if we want to disable it for HTTP, then why changing it for all TCP sockets?

[voxpelli]

I think it's enabled by default on Linux? My point is, if we want to disable it for HTTP, then why changing it for all TCP sockets?

Right, true, I can look into making a PR that only does it for the http-client, would relate to #31539

[voxpelli]

As #34185 is fixed by #42163 this PR no longer serves a purpose, closing it.