By default, postgrest-with-pgrst builds postgrest as a nix package,
which means that source changes cause a full rebuild. With this
change, running the loadtest as
PGRST_BUILD_CABAL=1 postgrest-loadtest
rebuilds directly using cabal, like postgrest-build. Note that
results between nix and cabal builds aren't necessarily comparable
due to differing build parameters.
This will prepare the pre-request statement (if prepared statements
are enabled) with a possible minor performance benefit -- the load
test is not entirely conclusive.
- switch from "unknown" parameter in text format to a "json" parameter in
binary format (no dependency update required)
- use a lazy bytestring "json" encoder (via updated hasql)
The configuration option db-pool-acquisition-timeout
specifies the time in seconds to wait for the pool to
free up a connection slot. Otherwise, a 504 error is
returned. By default, there is no timeout.
- hasql-pool-0.8
* 'release' now flushes the pool, and no longer destroys it;
'PoolIsReleasedUsageError' is gone compared to our fork
* now supports an acquisition timeout, which we don't use
yet
* lower bound on 0.8.0.2 to fix a Windows build issue
- hasql-1.6
introduces a position parameter to ServerError, which we
ignore
This version of hasql-pool is a simplified rewrite that doesn't use
the resource-pool package. The major API changes are that idle
connections are no longer timed out (and the corresponding setting
is gone), and that `release` makes the pool unusable, where it used
to remain usable and only flushed idle connections.
We depend on a PostgREST fork of 0.7.2 that gives us reliable
flushing, compare https://github.com/PostgREST/hasql-pool/pull/1
- hasql-pool 0.7 removes timing out of idle connections, so
this change removes the db-pool-timeout option.
Given that we were typically running with very high
timeout settings, I don't anticipate the lack of timeout
to introduce new issues, though we might want to consider
introducing some retry-logic down the line when we
encounter connection failures.
- See https://github.com/PostgREST/postgrest/issues/2422 for a
discussion on depending on a forked dependency. Besides adding
the dependency to the nix overlay, we're also adding it to
stack.yaml and a new cabal.project to allow stack/cabal users
to build the project.
The original test no longer makes sense once we drop pool timeouts
with the hasql-pool upgrade.
To somehow test that new connections have the settings, convert it
to flush the pool instead.
The idea is that this ensures
1. that it's easy for contributers to hack on PostgREST without
getting nix set up
2. we see things break more easily outside the very controlled nix
environment (e.g. it would be easy to add a GHC 9.4.2 build here)
This changes behaviour somewhat in that:
- We now consistently release the pool on shutdown, even on non-Unix
platforms, and including for CmdDumpConfig.
- We release the pool *after* interrupting `App.run`, which will
rather cause more than fewer connection to be closed properly.
(Previously any in-use connections would not have been caught by
`releasePool`, though *maybe* the `UserInterrupt` handling in
the web handler ends up closing the connections properly already
anyway).
(The main aim of the change is to make it clearer when and why the
pool is released.)
Previously, it was quite possible to have two connection workers
running, or to get into a state where a failed connection worker
is still considered running preventing new connection workers
from starting.
This is a bit ugly, it would be a bit nicer to just pass
'AppState.usePool appState' but then the types get messy.
Or we could introduce our own 'Pool' wrapper type.
The new tests verify that:
- statement_timeout on the authenticator role works to cancel slow statements
- changes to statement_timeout take effect on SIGUSR1
This reuses the old "limited_authenticator" role and adds some plumbing to
allow reliably changing the statement timeout even if the current role is
not functional due to a low statement timeout, and to make tests that modify
the role independent from each other.
- introduce module-wide metapostgrest fixture to have an out-of-band way to
manipulate the database, without having to spin up extra postgrest instances
per test
- reset statement_timeout at the start of the respective tests
- the isExecutable patch was broken, remove it
- the ncurses fix was broken, we still need `enableStatic`
- the original error was caused by the new ghc-bignum
package, which isn't a "proper" Haskell package; we filter
this out explicitly now
This pulls in an updated hlint version from nixpkgs, and fixes some
new eta-reduction warnings. (I don't particularly think this is more
readable, but I suppose being consistently hlint-clean is worth it.)
This patches static-haskell-nix to work for building postgrest
with updated nixpkgs (from ~202203):
- The ncurses 'enableStatic' argument doesn't exist anymore.
We use the vanilla package instead, which seems to work fine.
- The 'isExecutable' check fails with a strange error related
to trying to override 'mkDerivation'.
We patch 'isExecutable' to check explicitly whether we're building
postgrest. ('isExecutable' is used to determine whether to build
a package statically.)
* Write vegeta output to file explicitly.
The previous version using `tee` captured the standard
output of the `withPgrst` helper, too, causing weird
errors when those had unexpected output.
* Log and fail the postgrest build explicitly.
In situations such as changing nix dependencies, the
`cabal build` run could fail, but this wasn't visible
in CI.
With these changes, the load test now fails with a nice
error message when the build fails.
weeder needs to be compiled with the same GHC that we build postgrest
with.
Before this change, we were relying on the fact that we happened
to set ${compiler} to the same ghc8107 as used by nixpkgs for
haskellPackages, now we explicitly take it from the correct
haskell package set.
Robert Vollmert