add: use SO_REUSEPORT on platform supporting it - #4703
Conversation
c252511 to
27c16d7
Compare
|
This is failing all tests, I'd suggest to put these type of PRs as draft. |
Hmm... worked before latest push. Will switch to draft and fix. |
@steve-chavez - it looks like the issue is that on my machine PostgREST startup is fast enough so that it loads the schema cache before it accepts any requests. Here in CI the new instance fails with The question: is there any particular reason why we return |
We were aiming to have requests wait instead of 503, this waiting does happen during schema cache reload but not on startup; we discussed this on #4129. Would it be better to not listen on the socket? How would clients behave in this case? |
|
They would get a "connection refused" error, which means nobody there and is imo more confusing that any 5xx error. UX-wise I would prefer some waiting to a presumably hard fail any day. |
I am not convinced, see below. This is a complex topic so let's dig into it a little more. The startup sequence right now is:
The alternatives are:
b - listening on a socket only after schema cache loaded
So from the point of view of the clients (they don't know when Postgrest was started), we have 3 alternatives:
I am not sure what value clients get from the first two options comparing to the third one. Diagnostics and readiness checks should be done using In case of
So the first two options cause disruptions whereas the third one is fully zero-downtime and transparent to the clients. My take on it would be:
This would require splitting binding from listening on the main socket (ie. we need to bind without listening first so that we can pass the socket to the admin server). @steve-chavez @develop7 thoughts? |
|
Dependent on resolving (or having a workaround to) yesodweb/wai#853 |
64bb6ca to
2cb6503
Compare
Agree, sounds much better. |
7d7375a to
bd4c7ec
Compare
c12c6fd to
bec258c
Compare
5641f65 to
71ce2e3
Compare
steve-chavez
left a comment
There was a problem hiding this comment.
Besides the last nit on docs, this looks good to go. Great work! 🚀
|
Hm, for some reason I'm unable to merge. It says the branch is out-of-date with the base branch? |
Let me check if it is not multi-parent commit by accident. |
|
@mkleczek There was a hidden comment that was unresolved (had to click on |
Fixes #4694
Stacked on top of #5077 to make sure liveness and readiness checks are reliable.