This change introduces a new way to perform liveness check - instead of trying to open a TCP connection to main server, we check if main socket is listening and if main server accept loop thread is alive.
Opening a TCP connection in liveness check was problematic because:
* it used available file descriptors which might have been a problem under load
* made liveness check unreliable when multiple PostgREST instances are available on the same port (eg. using SO_REUSEPORT)
Right now code in App.run does not properly use bracket/finally to close sockets and clean-up mainSocketRef. This is not a big problem at the moment because the application is going to exit enyway but introducting proper resource handling will make future refactorings safer.
This change makes AppState and Listener modules independent from the way how application is terminated. It removes stateMainThreadId from AppState and introduces stateKillApp. It also removes exported function AppState.getMainThreadId replacing it with more general killApp.
This change makes Admin module independent from how liveness check is performed. It moves reachMainApp function to App module and makes Admin.runAdmin, instead of action to get the main socket, take an IO action checking liveness as a parameter.
Thanks to this, any change in how liveness check is performed and, more importantly, what data it uses, does not require changing multiple modules (Admin and App).
Issue: the example epoch date is wrapped in parentheses. This causes the server to return PGRST303: "The JWT 'exp' claim must be a number"
Fix: remove parentheses
Admin server was crashing silently and requests hang indefinitely.
With this fix, we are now logging the admin server exceptions and also
close the socket afterwards so admin request don't hang.
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
Instead of generating them at loadtest runtime, we generate these
dynamic targets once via Nix. Because we're using libfaketime, we're not
bound to doing it at the same time as running the tests.
This allows us to run each loadtest right after each other, with minimal
other stuff going on inbetween. It's also a tiny bit more reproducible,
because the loadtests against both branches run with exactly the same
set of targets, where they did not before.
Instead of building, running, building, running, ... we now build all
executables once ahead of time and then run all loadtests right after
each other. This can sometimes reduce noise when load on the GHA runner
varies over time.
Since this requires us to move building into the loadtest-against
script, it also allows to go back to have the regular postgrest-loadtest
command default to building with cabal for faster local iteration.
I'm not sure whether anyone is actually using them, but they never
worked really well, I believe. We should add something based on an
existing pre-commit hook manager, not that brittle custom thing we have.
Allows me to remove postgrest-with-git in the next commit instead of
duplicating it.
This is a heavily discouraged way of using these tools - they will
become outdated almost immediately and would need to be reinstalled on
every change to main.
This list is chronically outdated and hard to maintain with the two
column layout. Simplify and reduce, not important to keep up-to-date to
bring the point across.
We don't need to restrict us to run every request once. Instead we can
run as many as possible within 60s by removing the `-lazy` flag.
This also allows us to generate fewer targets, because vegeta will do
the repetition for us.
This test is not really functional (anymore?). It depends on some tokens
expiring - but they don't, really. This is because:
1) Before merging the RSA and HS variants earlier in this patch series,
they ran roughly 15s and 34s respectively in CI.
2) Since PostgREST takes a clock skew of up to 30 seconds into account,
there would be *at most* 4s of expiring tokens in a test run - and
*only* in the HS case, not for RSA.
While we didn't have much problem in this test, it's surely not a
problem either to make the test more reproducible with a fixed base
timestamp. This gives us a tiny bit less of a diff on the next change.
This test is not very interesting, really: It's hard to imagine to be
able to produce a regression that would slow down the no-cache case, but
would *not* show when a cache is used.
Let's use the same dependency when creating key material and when
creating tokens, no need to carry multiple different dependencies and
converting keys between them.
Instead of passing a path to the output target file, just pass the
directory in which to create the file. This allows the script to create
multiple files later.
When we changed the test structure to use libfaketime, we moved away
from generating targets as part of the chained command running
with-pg/with-pgrst. This function was left-over from that, the
genTargets script does not need to relay its remaining arguments
arguments anymore.
Instead of creating separate test suites for the key type, the PostgREST
instance now accepts both keys via a JWKSet and the targets are
generated 50/50 for both.
The results are still reported seperately by using a different URL,
which shows up as separate rows in the results.
The script currently depends on results from "head" and "main" to be
present to calculate the change ratio. Fallback nicely in case these are
not available, which allows using the same to report URL-by-URL stats
locally.
BREAKING CHANGE
Breaks the string comparison operators implemented in #3813. Those can
be replaced with regex searches using JSON Path `search()` function.
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
steve-chavez