Before 9.12.1 it was necessary to mark functions as INLINABLE or INLINE to make GHC consider cross-module specialization of polymorphic functions.
9.12 added a new -fexpose-overloaded-unfoldings flag that exposes optimized polymorphic functions in interface files. -fspecialise-aggressively then makes GHC apply aggresive specialization.
This change ensures PostgREST starts listening on a server socket only after it loaded the schema cache and is ready to handle requests. It is no longer going to return 503 errors during startup until the schema cache is loaded.
Right now stateNextListenerDelay is only used in Listener module.
This change is a small refactoring moving nextListenerDelay to Listener to increase cohesion and decrease coupling.
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
```
retryingSchemaCacheLoad should not clear existing schema cache upon failure - there is no reason to do that. If there is a communication issue with the database server or db is down, clients are going to get 502 anyway. If it was a glitch when loading the schema cache - the clients are going to use old (stale) schema cache for some time until next retry re-loads it successfully.
There was an oversight on d556cea, `timeout_authenticator` doesn't have
any privileges on the tables so it shouldn't be making requests to the
API server or it will end up with 401 responses.
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!
Replaces the `Maybe [Text]` with `[Text]`. The `Maybe` is unnecessary
because we handle `Just []` and `Nothing` the same way.
Signed-off-by: Taimoor Zaeem <taimoorzaeem@gmail.com>
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 also requires enabling shared_preload_libraries=pg_stat_statements.
Really we only need compute_query_id=on, but pg_stat_statements is
useful for other tests and it already enables compute_query_id (since
it's `auto` by default).
The PGRST000 database connection error message was "Database connection
error. Retrying the connection.", but reconnection attempts are already
logged separately by the reconnection observation, and on fatal errors
(e.g. authentication failure) PostgREST does not retry at all. Drop the
"Retrying the connection." part, leaving "Database connection error.".
The config dump in Config.hs listed db-config, db-pre-config and the
db-pool-* settings out of alphabetical order, while the rest of the dump
was sorted. Now the whole dump is consistently alphabetical and update
the expected IO test configs to match.
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.
Currently, authentication and response execution each unwrap ExceptT with separate runExceptT calls, which split the main request flow across nested pattern matching and Either handling. Control flow is complex and difficult to understand.
The goal of this change is to make request execution as sequential
monadic code with clear error handling.
To implement that, request handling is now run in ExceptT over WriterT (Last ByteString) IO monad stack. Auth role is written after authentication succeeds and further returned along the response. Thanks to it response observation generation is centralized at the end of request handling.
It was necessary to abstract monad stack in getAuthResult, lookupJwtCache, postgrestResponse, and withTiming to enable introduction of WriterT.
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.
