eio
eio copied to clipboard
net: add somaxconn to Eio.Net
Can you clarify why we need this ?
Most systems will just silently cap if you go over the max, so if this is important they can just pass a big number to listen, it also doesn't have a huge implication on performance, if it does we're not calling listen fast enough and we should lose connections anyway.
Mostly following lwt here (https://github.com/ocsigen/lwt/blob/master/src/unix/lwt_io.ml#L1604) as I am looking at the eio
version of establish_server
.
Most systems will just silently cap if you go over the max, so if this is important they can just pass a big number to listen, it
Yes. This PR attempts to be accurate with the "big number" by default so the user of eio
doesn't have to guess.
Ci is failing due to OCaml 5 issue I think - (https://ci.ocamllabs.io/github/ocaml-multicore/eio/commit/619f75f961e27a897ab37a5b433688d01c38feb8/variant/(lint-doc)).
@talex5 any suggestions on getting the CI green?
I'm not sure defaulting to a large number is even useful. https://serverfault.com/a/519152 says:
Sometimes it's preferable to fail fast and let the load-balancer to do it's job(retry) than to make user wait - for that purpose we set net.core.somaxconn any value, and limit application backlog to e.g. 10 and set net.ipv4.tcp_abort_on_overflow to 1.
https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-listen says:
When calling the listen function in a Bluetooth application, it is strongly recommended that a much lower value be used for the backlog parameter (typically 2 to 4), since only a few client connections are accepted. This reduces the system resources that are allocated for use by the listening socket. This same recommendation applies to other network applications that expect only a few client connections.
So probably better to make the application choose a value.
yes please, don't default to max, I think forcing the user to pass the value was fine.
@talex5 @haesbaert I have removed somaxconn being the default value for listen ~backlog
now.
@haesbaert @talex5 If there are no objections could you please approve/merge this PR.
If there are no objections could you please approve/merge this PR.
But what problem is this solving? So far the only explanation is "Mostly following lwt here", which doesn't seem very convincing. I suggest closing this until we find a use for it.
Yes, there is no direct or pressing need for this PR; so closing this now.