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.
The dependency on it was removed in #3608 already, but we forgot to
remove it from postgrest.cabal, which caused it to still be built.
We didn't realize because all references of it were stripped away by
dead code elimination anyway.
Previously, the loadtest would always compare against main and the
latest tag. This meant a loadtest running on the v13 release branch,
would compare to a *future* version of both. This is not useful, and
also not supported by `postgrest-loadtest-against`, which recently
introduced a change on `main`, that now breaks the tests on the back
branches.
With this change, the loadtest will always run against the current
target branch of a PR, so against the v13 branch for a PR against v13,
for example. Also, it will compare against the latest released tag *for
that branch*.
Thus, when running this for v12, it will test against the v12 branch and
the v12.2.12 tag.
Splitting the executable in a separate output avoids distributing all of
the libraries and documentation, which are not needed when just running
PostgREST.
Reduces closure size from 4.3G to 73.9M for the flake exported packages.
Resolves#4149
Moat has been bought by Oracle. The advertising subpage redirects to
oracle.com, but pointing at that would be a bit misleading.
No need to keep failing links either.
Wolfgang Walther