fix: shutdown should wait for in flight requests

[mkleczek]

DISCLAIMER:
This commit was authored entirely by a human without the assistance of LLMs.

Fixes #4689

[mkleczek]

Marked it as draft because there is a problem caused by yesodweb/wai#853.
What that means is we have to put the hard limit on gracefulShutdownTimeout otherwise warp will not shutdown at all if there is a reverse proxy keeping connections open (ie. using HTTP keep-alives).
@steve-chavez we need to decide what this limit should be. Unfortunately, warp is going to wait exactly that long if there are any keep-alive connections, so this timeout cannot be too long.

[mkleczek]

Related: yesodweb/wai#1064

[mkleczek]

@steve-chavez @wolfgangwalther - this is dependent on warp fixing yesodweb/wai#853. I've raised a draft PR yesodweb/wai#1064 but don't know if or when it will land.

First commit in this PR is a change to our Haskell overlay pointing to warp version from the above PR.

We need to decide what to do with this one.

[steve-chavez]

this is dependent on warp fixing yesodweb/wai#853. I've raised a draft PR yesodweb/wai#1064 but don't know if or when it will land.

Nice, looks like the maintainer is already replying.

First commit in this PR is a change to our Haskell overlay pointing to warp version from the above PR.
We need to decide what to do with this one.

Since replies are fast I think we should aim to merge upstream and not fork/vendor for now.

[wolfgangwalther]

But there are cabal build failures caused by some mysterious compilation errors when building jose-jwt. I am not sure what's going on but new warp introduces crypton >= 1.1 dependency (indirectly) which might be incompatible with jose-jwt.

I believe these mysterious compilation errors should be fixed with #4825 - at least I had some of these and I assume they were the same as yours :)

[Vlix]

I am not sure what's going on but new warp introduces crypton >= 1.1 dependency (indirectly) which might be incompatible with jose-jwt.

If you use crypton-x509-1.8.0, it should work with crypton < 1.1. (haven't tried building the new warp with crypton-x509-1.8.0, so I can't guarantee it works)

[steve-chavez]

It also looks like newer warp introduces some increased memory usage.

I see the failures:

not ok 7 - POST /rpc/leak?columns=blob: with a json key of 50M the memory usage(77,356,040 bytes) is more than 73M
not ok 8 - POST /leak?columns=blob: with a json key of 50M the memory usage(77,239,768 bytes) is more than 73M
not ok 9 - PATCH /leak?id=eq.1&columns=blob: with a json key of 50M the memory usage(77,304,464 bytes) is more than 73M

@mkleczek I think it should be fine to correct these lines to 78M.

postgrest/test/memory/memory-tests.sh

Lines 113 to 115 in 6b4022b

	jsonKeyTest "50M" "POST" "/rpc/leak?columns=blob" "73M"
	jsonKeyTest "50M" "POST" "/leak?columns=blob" "73M"
	jsonKeyTest "50M" "PATCH" "/leak?id=eq.1&columns=blob" "73M"

[mkleczek]

@mkleczek I think it should be fine to correct these lines to 78M.

Done

[steve-chavez]

🚀 🚀 🚀

[postgrest-ci]

Backport failed for v14, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin v14
git worktree add -d .worktree/backport-4702-to-v14 origin/v14
cd .worktree/backport-4702-to-v14
git switch --create backport-4702-to-v14
git cherry-pick -x baebacf3db76594a7799d3f76adad359f3fa2e25

[steve-chavez]

It's not possible to backport this because it depends on

• 051b479
  • which depends on dropping pg 13 support 1b74b93

There will be a showstopping error:

       error: postgresql_13 has been removed since it reached its EOL upstream

[steve-chavez]

WDYT of changing this one to a feat instead of fix, that way we add docs for it. Some projects have "graceful shutdown" documented like:

• https://trino.io/docs/current/admin/graceful-shutdown.html
• https://static-web-server.net/features/graceful-shutdown/

We can mention this is done by Warp.

[wolfgangwalther]

In #4902 (comment), we noticed that this also makes these kinds of requests a tad slower. Most likely introduced via one of the changed dependencies.