running postgrest-loadtest with artificial latency #2682
Conversation
robx
changed the title
This pipes data between client/postgrest and between postgrest/database through a proxy that adds delay (github.com/robx/slocat).
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.
|
I also pulled in the build-with-cabal option here. |
| $PGRST_CMD ${legacyConfig} > "$tmpdir"/run.log 2>&1 & | ||
| pid=$! |
There was a problem hiding this comment.
Before the change to nix-build we had some more magic here to get the correct pid for cleanup, see your first commit around this area: 0c5d2e5
Aren't we back to getting the cabal pid, or even worse the bash pid from postgrest-run, which will prevent stopping postgrest properly?
There was a problem hiding this comment.
When PGRST_BUILD_CABAL isn't set, nothing changes. Otherwise, it's cabal's job to deal with its children, which it should do properly in recent releases (not entirely sure whether nixpkgs has caught up there).
| if [ -z "''${PGRST_BUILD_CABAL:-}" ]; then | ||
| echo -n "Building postgrest (nix)... " | ||
| nix-build -A postgrestPackage > "$tmpdir"/build.log 2>&1 || { | ||
| echo "failed, output:" | ||
| cat "$tmpdir"/build.log | ||
| exit 1 | ||
| } | ||
| PGRST_CMD=./result/bin/postgrest | ||
| else | ||
| echo -n "Building postgrest (cabal)... " | ||
| postgrest-build | ||
| PGRST_CMD=postgrest-run | ||
| fi |
There was a problem hiding this comment.
Is PGRST_BUILD_CABAL really the right term? nix-build does nothing different - it builds with cabal, too.
The difference seems to be in:
- Which dependencies are used, so in which nix environment is the command run? This branch or
main? - Is cabal's build cache re-used? The nix command probably does not, but
postgrest-builddoes, right?
I think PGRST_BUILD_CABAL is misleading. It's actually going back to using the current nix environment, so the dependency problem would still be present with that setting, correct?
Overall, I think the fix in 0c5d2e5 was just not an improvement and we should revert it.
The suggestion I made in #2308 (comment) seems better. The loadtest-against / with-git infrastructure was designed to always use the current's branch nix environment on purpose. This does not cover some edge-cases, like the one you found - but it gives us some nice other properties, namely that we can update our tooling (nix commands for loadtest etc..) - and still run the new tools on older branches. This was necessary when I once ran loadtest through some months/years of commits to see patterns: #1812 (comment)
There was a problem hiding this comment.
I don't particularly care what happens to this change as such -- I needed it to do my work here.
(It's unfortunate that our nix tooling is as rigid and obstructive as it is, it would be much preferable if changes such as this weren't needed. Ideally, it were possible to iterate quickly with local changes to the build scripts, but that's not where we're at.)
This pipes traffic between the client and postgrest and between postgrest and postgresql through a helper tool (https://github.com/robx/slocat) that adds a configurable amount of latency. I'm not suggesting this be merged, just filing it here for discussion at #2295.
This can be used like