This reports the percentage change between the current head branch and
the main branch, which is exactly the number we'll want to make our
decisions on "success or fail" on.
CI failures will initially be reported for regressions of 5% or more on
an individual number.
Because we separate loadtest results per URL now, we can move the error
tests into the regular mixed bag of loadtests - we will be able to tell
from the misspelled URLs when we hit a regression in that area.
We should be able to do similar things for JWT tests, but we'll need
more infrastructure here.
We previously used "rate", i.e. number of requests per second, as the
primary metric to judge loadtest results. However, this has always been
varying from run to run quite a bit, especially in CI where other jobs
possibly run on the same VM host.
The run-to-run variance has massively increased after splitting the
results up per request. Example run in CI with rate on the PR
introducing this change (on which we would expect no change at all):
| rate [1/s] | main | head | Δ |
|:-----------------------------------|-------:|-------:|-----:|
| / | 9.4 | 9.5 | 1% |
| /actors | 870.4 | 1023.0 | 18% |
| /actors?actor=eq.1 | 188.5 | 198.6 | 5% |
| /actors?actor=eq.1&columns=name | 197.3 | 167.1 | -15% |
| /actors?select=*,roles(*,films(*)) | 153.9 | 144.9 | -6% |
| /films?columns=id,title | 157.9 | 182.6 | 16% |
| /films?columns=id,title,year,... | 87.0 | 87.1 | 0% |
| /roles | 204.5 | 267.3 | 31% |
| /rpc/call_me | 231.3 | 208.8 | -10% |
| /rpc/call_me?name=John | 212.2 | 201.7 | -5% |
From the data we can easily tell that the very reason that rate as a
paramter has only worked, so far, because the data was *heavily*
dominated by the requests on the root endpoint for OpenAPI. The longer
duration makes the request much less vulnerable for concurrent activity.
For all other requests its essentially not possible to judge the effect
of a PR this way.
One way to counter this would be to massively increase the time the
loadtest runs. More samples will result in a smoother average. However,
that's not practical for usability of CI. In the original PR #1812 I
already evaluated using the *minimum latency* as the most reliable
criteriumi, but this has never really caught on. The theory behind this
is: The variation in timings between requests is happening because of
concurrent activity, priority chosen by the scheduler, availability of
resources and such - all factors *outside* our control, and *irrelevant*
to the Haskell code we're writing.
Using the minimum latency is an estimation of how fast the code can run
*in the best case*. This might not be a number relevant for production,
but it's much more directly related to the code we write.
Here's to show how variation becomes *much* smaller with minimum latency
as the parameter:
| min latency [μs] | main | head | Δ |
|:-----------------------------------|---------:|-------:|-----:|
| / | 1275.3 | 1263.6 | -1% |
| /actors | 10.0 | 9.9 | -1% |
| /actors?actor=eq.1 | 50.7 | 48.3 | -5% |
| /actors?actor=eq.1&columns=name | 54.1 | 54.0 | 0% |
| /actors?select=*,roles(*,films(*)) | 63.2 | 61.9 | -2% |
| /films?columns=id,title | 51.1 | 50.7 | -1% |
| /films?columns=id,title,year,... | 121.9 | 121.8 | 0% |
| /roles | 42.9 | 42.6 | -1% |
| /rpc/call_me | 45.6 | 45.4 | 0% |
| /rpc/call_me?name=John | 44.4 | 44.2 | 0% |
Since we're separating results per request now, we can only sensibly
focus on *one* parameter - otherwise this would get really clunky
UI-wise. Especially for automated CI failures, minimum latency is the
logical choice.
This commit starts using minimum latency, i.e. P0, but any percentile
should be an improvement over the status quo. A later commit will change
to a different P-value.
Different requests hit different code paths and perform very
differently. By looking at each request type separately, we should be
able to get a much better idea of what kind of change in performance
we're looking at and where the root cause might be.
It will hopefully also allow us to migrate some of the other test-cases
into the main loadtest.
Ultimately, we only look at the `rate` column, so we can just as well
remove all other columns.
This makes the next step, when we split results by request type, much
less noisy.
Instead of taking wild guesses at the runtime of the target generation
itself, we're just making sure to reset the system time to a fixed value
when we ultimately start PostgREST. This allows us to create the right
JWT expiry values ahead of time.
This was required for v9 and earlier, but these don't build with the
current nix invocation anymore anyway. Even loadtesting against v10 does
not work, because `--version` is used in one of the wait scripts and
this was only added in v11.2.
So no need to pretend we'd support comparing against older versions.
Disabling the autovacuum daemon should also help reproducibility in
theory, although I don't know of any cases where we hit a problem with
that.
VACUUM changes the order of rows that PostgreSQL returns for some table
without explicit ordering, thus doing the latter to make it consistently
reproducible.
After ANALYZE estimates are 100% exact for the moment, so some requests
which returned 206 Partial Response now return 200 instead. The fact
that PostgREST returns 206 on an unfiltered endpoint can probably be
considered a bug.
Proves the failure on https://github.com/PostgREST/postgrest/issues/4622.
This doesn't require additional test infra, only nginx. Taking advantage
of the `stream {}` context which is also compatible with unix socket
besides TCP.
So we can do `PGRST_DB_ANON_ROLE="" postgrest-run` since empty string has
meaning. argbash conflates both unset and empty string so this requires
a workaround.
PostgREST failed when querying role settings where current
role name contained uppercase letters. This commit resolves
it by quoting the CURRENT_USER.
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
- Create separate test suite for observability tests
- Create wrapper script `postgrest-test-observability`
- Add to CI and `postgrest-check`
- Move JWT cache tests under observability tests
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
Chaining the target generation command after the build is done ensures
that if a build takes too long, the dynamic target generation for vegeta
won't cause 401 Unauthorized errors due to already expired JWTs.
Also split the rsa materials writing to another python program for
easier maintenance.
Currently the `postgrest-loadtest -k jwt-*` loadtests use the OPTIONS method,
which is good for isolating the perf changes for JWT auth.
But using GET is more accurate for actual perf improvements,
since it actually hits the database.
This adds a method argument for postgrest-loadtest, so GET can be used like:
```
postgrest-loadtest -k jwt-hs --method GET
```
CI is not changed, this is mostly useful for manually checking variance.
Renames these scripts to `postgrest-with-pg-xx`. The renaming
helps reduce the length of temporary filenames. This is needed
to ensure that socket file names remain under the maximum
allowed length of 107 chars.
Closes#4461.
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
* replaces v2-build -> v2-run (which should be v2-exec)
with one-shot v2-run
* fixes empty postgrest.prof file by sending SIGINT to the profiled
postgrest process to trigger writing it explicitly
Changes:
1. Refactoring and some cleanup of JWT handling code:
* Instead of caching AuthResult cache decoded claims (which signature was verified). Validating claims and determining role is done after cache lookup
* Cleaned up API so that usage of it is simplified: lookupJwtCache cache key >>= parseClaims configJwtAud time
* Handling of JwtCacheState initialization and updates of configuration is encapsulated in Auth.JwtCache module
2. Generic high performance (hopefully) scalable, dynamically resizeable cache implementation based on stm, stm-hamt and sieve algorithm. It also integrates with PostgREST measurements infrastructure providing usage stats (ie. hit ratio, evictions count)
Completes the jwt loadtests, now we have non-cached, cached + worst case
for the cache.
- jwt-hs (edited): now has the cache disabled
- jwt-hs-cache: cache enabled for jwt-hs
- jwt-hs-cache-worst: worst case of the jwt-hs cache
- jwt-rsa (edited): now has the cache disabled
- jwt-rsa-cache: cache enabled for jwt-rsa
- jwt-rsa-cache-worst: worst case of the jwt-rsa cache
Also deletes `nix/tools/generate_targets_rsa.py` and uses a single
python script.
Should prove what's mentioned on
https://github.com/PostgREST/postgrest/pull/4084#issuecomment-2998170423
Wolfgang Walther