test: add loadtest for async purge of JWT cache #4034
Conversation
| # We want to ensure 401 Unauthorized responses don't happen during | ||
| # JWT validation, this can happen when the jwt `exp` is too short. | ||
| # At the same time, we want to ensure the `exp` is not too big, | ||
| # so expires will occur and postgREST will have to clean cached expired JWTs. | ||
| def estimate_adequate_jwt_exp_increase(iteration: int) -> int: | ||
| # estimated time takes to build and run postgrest itself | ||
| build_run_postgrest_time = 2 | ||
| # estimated time it takes to generate the targets file | ||
| file_generation_time = TOTAL_TARGETS//(10**-5) | ||
| # estimated exp time so some JWTs will expire | ||
| dynamic_exp_inc = iteration//1000 | ||
|
|
||
| return build_run_postgrest_time + file_generation_time + dynamic_exp_inc |
There was a problem hiding this comment.
This is the main addition in comparison to #4023
There was a problem hiding this comment.
Do we account for the clock skew toleration we have built in here?
There was a problem hiding this comment.
The clock skew is only honored at the time of insertion into cache. We don't take it into account when we lookup the cache.
There was a problem hiding this comment.
Yeah, I realized that, too and asked the same question in the cache PR. I'm not sure whether that's the correct thing to do, though.
There was a problem hiding this comment.
@taimoorzaeem That looks like the right behavior.
So the clock skew is only considered at the JWT validation phase but not during cache invalidation phase.
There was a problem hiding this comment.
I'm not sure whether that's the correct thing to do, though.
Yeah on second thought, doesn't make sense to slow down requests from "still valid" JWT caches.
There was a problem hiding this comment.
I wonder if we should make the skew configurable, so we can output it with dump-config then we could also use it as parameter of this loadtest.
That's something that can be done in another PR though.
There was a problem hiding this comment.
I wonder if we should make the skew configurable, so we can output it with dump-config then we could also use it as parameter of this loadtest.
Opened #4035
steve-chavez
force-pushed
the
test-purge-fix
branch
from
a0274d4 to
9b0707e
Compare
|
Shoot, forgot the migrations and copy/pasted the wrong results. Will fix. Edit: Done. |
steve-chavez
force-pushed
the
test-purge-fix
branch
from
9b0707e to
c087d79
Compare
|
Results showing on https://github.com/PostgREST/postgrest/actions/runs/14562788036?pr=4034#summary-40848158792, head/main show better throughput than v12.2.10 but maybe that's just variance. Let's see what it shows when merged. |
|
Results are maintained: https://github.com/PostgREST/postgrest/actions/runs/14562931741#summary-40848497694 I wonder what has improved on main that makes the jwt results better. |
It could be the change from jose to jose-jwt on the haskell side? |
That looks like it! Also the increase in perf on latest CI runs looks to be around |
The JWT Cache gains are still noticeable, but less than before. On v12.2.10 gains are Leaving numbers here just in case. With https://github.com/PostgREST/postgrest-benchmark, I ran: # stable with cache enabled and disabled, on each one doing a loadtest without JWT to get max reqs and another one with same JWT
postgrest-bench-deploy && \
postgrest-bench-k6 10 k6/GETSingle.js && postgrest-bench-k6 10 k6/GETSingleJWT.js && \
PGRSTBENCH_JWT_CACHE_ENABLED="false" postgrest-bench-deploy && \
postgrest-bench-k6 10 k6/GETSingle.js && postgrest-bench-k6 10 k6/GETSingleJWT.js && \
# devel with cache enabled and disabled, on each one doing a loadtest without JWT to get max reqs and another one with same JWT
PGRSTBENCH_JWT_CACHE_ENABLED="true" PGRSTBENCH_USE_DEVEL="true" postgrest-bench-deploy && \
postgrest-bench-k6 10 k6/GETSingle.js && postgrest-bench-k6 10 k6/GETSingleJWT.js && \
PGRSTBENCH_JWT_CACHE_ENABLED="false" PGRSTBENCH_USE_DEVEL="true" postgrest-bench-deploy && \
postgrest-bench-k6 10 k6/GETSingle.js && postgrest-bench-k6 10 k6/GETSingleJWT.jsResultsbuilding all machine configurations... warning: you did not specify '--add-root'; the result might be removed by the garbage collector client............> copying closure... pg................> copying closure... pgrst.............> copying closure... client............> copying 0 paths... pg................> copying 0 paths... pgrst.............> copying 0 paths... postgrest-bench> closures copied successfully pg................> updating GRUB 2 menu... client............> updating GRUB 2 menu... pgrst.............> updating GRUB 2 menu... client............> activating the configuration... pg................> activating the configuration... pg................> setting up /etc... client............> setting up /etc... pgrst.............> activating the configuration... pg................> reloading user units for root... client............> reloading user units for root... pgrst.............> setting up /etc... client............> restarting sysinit-reactivation.target pg................> restarting sysinit-reactivation.target pgrst.............> reloading user units for root... pg................> activation finished successfully client............> activation finished successfully pgrst.............> restarting sysinit-reactivation.target pgrst.............> activation finished successfully postgrest-bench> deployment finished successfully |
3 participants
Closes #4001.
This proves the fix on #3889, while running on v12.2.9 the loadtest doesn't even finish all the generated targets (50000) and has poor throughput, while HEAD will finish and show better throughput.
This has to be proved locally now since we already merged #3889 and released it, but it's very clear:
Only
12078requests done in v12.2.9 and50001(all of them) on HEAD.