nix(loadtest): reduce JWT variants to a single jwt-cache test#5030
Conversation
wolfgangwalther
force-pushed
the
reduce-loadtests
branch
from
79e5434 to
c053999
Compare
That is surprising 😮. Does |
The link you quoted is for a run without the changes in this PR. So far, |
Thanks, got it. I can see the 0 on the |
The script currently depends on results from "head" and "main" to be present to calculate the change ratio. Fallback nicely in case these are not available, which allows using the same to report URL-by-URL stats locally.
This is not used by CI and I doubt it's actually used by anyone locally.
The public key is not needed for target generation, only the private key, so it's enough to pass that.
This makes the next change easier to do, but should be easier to review step-by-step.
Instead of creating separate test suites for the key type, the PostgREST instance now accepts both keys via a JWKSet and the targets are generated 50/50 for both. The results are still reported seperately by using a different URL, which shows up as separate rows in the results.
When we changed the test structure to use libfaketime, we moved away from generating targets as part of the chained command running with-pg/with-pgrst. This function was left-over from that, the genTargets script does not need to relay its remaining arguments arguments anymore.
Instead of passing a path to the output target file, just pass the directory in which to create the file. This allows the script to create multiple files later.
Let's use the same dependency when creating key material and when creating tokens, no need to carry multiple different dependencies and converting keys between them.
Rethrowing the same error, but without stacktrace is not really useful, but only makes the code (and debugging!) harder than it should be.
Just a bit simpler.
Merge the same code into genTargets, which removes the need to save the private key to disk and read it again.
This test is not very interesting, really: It's hard to imagine to be able to produce a regression that would slow down the no-cache case, but would *not* show when a cache is used.
While we didn't have much problem in this test, it's surely not a problem either to make the test more reproducible with a fixed base timestamp. This gives us a tiny bit less of a diff on the next change.
This test is not really functional (anymore?). It depends on some tokens expiring - but they don't, really. This is because: 1) Before merging the RSA and HS variants earlier in this patch series, they ran roughly 15s and 34s respectively in CI. 2) Since PostgREST takes a clock skew of up to 30 seconds into account, there would be *at most* 4s of expiring tokens in a test run - and *only* in the HS case, not for RSA.
We don't need to restrict us to run every request once. Instead we can run as many as possible within 60s by removing the `-lazy` flag. This also allows us to generate fewer targets, because vegeta will do the repetition for us.
wolfgangwalther
force-pushed
the
reduce-loadtests
branch
from
c053999 to
17773e5
Compare
2 participants
Working towards #5015, specifically addressing #5015 (comment).
Now, I wanted to "save" the
-cache-worstvariants of the tests, merging them with the regular-cachevariants. But ultimately I realized that the-cache-worsttests were broken, at least for quite some time. The commit message of 7e6650b has the details.I still kept both
HS256andRS256algorithms for the cache test, but merged them into a single test suite. They should be reported via different URLs separately.Overall simplified the target generation stuff a lot.
TODO:
[ ] Fix CPU / MEM reports. This is a pre-existing problem, which was introduced with the(See nix(loadtest): fix cpu/mem monitoring for tests using libfaketime #5044)libfaketimechange. Now, the metrics are for thefaketimebinary, notpostgrest, which is not really helpful.