There is no need to store the out-links to the various build artifacts
in the current working directory while running postgrest-with-pg, which
is used by postgrest-loadtest. Instead reference the nix store paths
directly.
Resolves#5028
This is useful when multiple instances run on the same machine, for
example behind a proxy. Unix sockets for web and admin servers can then
be put in the same folder for each instance.
Can be helpful when writing tests as well.
The `if response` check a few rows down would not actually work without
initializing the variable first. It'd throw:
```
E UnboundLocalError: cannot access local variable 'response' where
it is not associated with a value
```
ghcr does not support uppercase letters, which was not a problem when
testing in my fork. But the PostgREST org's name obviously has uppercase
letters in it, so we need to lowercase these for the ghcr namespace.
This reverts commit 840d3f99ed.
This was a nice idea in theory, but in practice this means that the
build jobs do not run at all on the default branch anymore. This means
the caches they push to won't be used by any other jobs, neither on the
tag pipelines, nor on PRs. This in turn makes all of these jobs really
slow.
Not helpful!
Previously this would only fail if *each* row had `:x:` in it, which was
never the case, because the header never has it. It is supposed to fail
when *at least one* row has an ❌.
This was brought over in the last commit, because the postgis was
re-using the same config as another test. Now it has a separate config,
so we can reduce it a bit.
Previously, information about each test-suite was repeated in 3 separate
places:
- as a label and as implicit knowledge in the test-suite itself,
- as a comment in Main.hs, and
- as a configuration in SpecHelper.hs.
With this change, there will be a single source of truth in the test
suite itself. This will allow a single test-suite to easily test
multiple different configurations.
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.
No need to do this in as many jobs. Splitting all the github stuff from
docker stuff into two jobs is enough. This still allows to conditionally
enable docker jobs in contributors repos, depending on whether the
relevant docker credentials are provided - but avoids using too many
concurrent runners.
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.
No need to spin up full VM runners for small automation tasks, when we
can use single-CPU runners in containers instead.
https://docs.github.com/en/actions/reference/runners/github-hosted-runners#single-cpu-runners
(some of this will potentially not work, because dependencies in the
slim image might not be available - however, it makes no sense to create
this as a PR, because all jobs touched here run on branches only. Thus
pushing directly to main)
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.
The most likely offender to fail early is jwt-rsa-cache-worst - which
then cancels *all* loadtests at once. This is unnecessary churn for CI.
Instead we can re-run only that one test later.
These fail with orioledb, because it randomly returns a different order.
Of course, that's allowed - ordering is not guaranteed unless specified.
We should write our tests with more explicit ordering in general, even
if multiple major versions show the same behavior.
This adds a CI job to run our latest postgrest version against the NixOS
VM test currently available in Nixpkgs. Now, this will not always be in
sync, so has the potential to be failing. However, since the Nixpkgs VM
test is really simple, this should only happen when we introduce a
breaking change on a very fundamental level. The failing test will then
be resolved once the new version is available in Nixpkgs and we have
updated our lock file.
This is essentially just a sanity check to make sure we're not breaking
something fundamentally - and if we do, it's a head up for me to adjust
the Nixpkgs tests accordingly. Those might otherwise break unnoticed
since Nixpkgs does not have a good notification system for such
breakages in place.
We do use the chance to run the static executable in this test, which
was previously not tested at all.
It also gives us a first test whether NixOS VM tests work well in GitHub
Actions.
I doubt that anyone would use cabal2nix or update-nix-fetchgit manually
- and at the same time everyone surely has git installed through their
OS anyway, otherwise they can't even get that far.
There is no point to actually realize the derivations to disk, when they
are in cache. `nix-build-uncached` checks the cache and stops before
downloading dependencies if they are available. It will however build as
normal on a cache-miss. Should speed up CI when no Nix dependencies
change a fair bit.
We can only do this for the MacOS job, which only tests the builds. We
can not do this for other jobs which require the build results to be
present before uploading parts of them as artifacts.
These should have been updated by postgrest-nixpkgs-upgrade, when I
upgraded the Nixpkgs pin, but I did not follow my own readme, and just
ran nix flake update...
My recent commit to fix the stack cache on macOS only changed the
restore keys, but no cache would ever be saved that way. I effectively
disabled all caching...
Unfortunately, `docker manifest create` does not support image digests,
so we need to push a tag for each version of the image. Furthermore, the
docker CLI does not provide a way to delete that tag afterwards, so we
will end up with those temporary tags.
We now have two stack jobs for macOS, but they use the same cache, which
is very inefficient. Adding the architecture to the cache key will give
each of these jobs a separate cache.
Jumping past GHC 9.6, 9.8 and 9.10 immediately, because these versions
do not successfully build the static binary with TemplateHaskell and
iserv-proxy, yet.
Michal wrote the changes to hsie.
Co-authored-by: Michal Kleczek <michal@kleczek.org>
These don't really make sense, because it's non-trivial to construct a
build plan that *ensures* the dependency that renovate creates a PR for
is tested with the latest version. Cabal might just select a build plan,
that doesn't include the dependency, in which case we could just as well
have no upper bounds at all.
Since there is no good way to test these changes (I tried, a while
ago!), let's just disable these PRs. They are more annoying than
anything else.
I'm not sure whether that's Lix specific, but when I enter nix-shell, I
get quite a long TMPDIR, which results in all postgrest-with-pg-XX
scripts failing, because the unix socket path is too long. If TMPDIR was
not set, mktemp would fall back to /tmp, so we can just do that in any
case, which guarantees a short path in every case.
The x86 GitHub runner will not be available anymore, soon.
We might be able to re-introduce this, once we can build a static
executable via Nix on darwin, too.
This way we benefit from NixOS' binary cache to deliver GHC for us and
don't need to cache it ourselves.
This will become relevant once we do that for more platforms.
We have fixed the static build of PostgreSQL upstream, so we don't need
our separate overlay anymore. One more step towards building a static
executable on other platforms.
Instead of rolling our own, we can use some tooling from nix / nixpkgs.
We drop the "statically linked" check, because our goal is to compile
mostly-static executables to darwin, too. However, those will never be
fully static, because they always link to the platform's libc.
Wolfgang Walther