Throughput slightly lower (~7%) on new JWT cache

[steve-chavez]

Problem

Looks like there's a slight decrease on the new SIEVE JWT cache, the jwt-hs-cache loadtest shows:

• v13.0.4: 8188.433272623855
• head: 7645.6599188352375

Not a big deal since the loss is slight and the new JWT cache is much better in CPU/MEM usage, but perhaps it can be improved?

[mkleczek]

@steve-chavez - are we sure this is caused by cache implementation and not by something else?
Will try to look at it in the next couple of days.

[steve-chavez]

@mkleczek Yes, the drop occurred on the SIEVE PR #4084 and can be seen on jwt-rsa-cache and jwt-hs-cache (see throughput numbers)

[mkleczek]

@steve-chavez
I get the following error when trying to run the load tests locally:

[nix-shell:~/Development/postgrest]$ postgrest-loadtest
postgrest-with-postgresql-17: You can connect with: psql 'postgres:///postgres?host=/private/tmp/nix-shell-20426-0/postgrest/postgrest-with-postgresql-17-NP7/socket' -U postgres
postgrest-with-postgresql-17: You can tail the logs with: tail -f /private/tmp/nix-shell-20426-0/postgrest/postgrest-with-postgresql-17-NP7/db.log
delaying data to/from postgres by 0ms
Building postgrest (nix)... done.
Invalid argument `./result-bin/bin/postgrest'

Usage: postgrest [-v|--version] [-e|--example] [--dump-config | --dump-schema] 
                 [FILENAME]

  PostgREST 12.3 (pre-release) / create a REST API to an existing Postgres
  database

Available options:
  -h,--help                Show this help text
  -v,--version             Show the version information
  -e,--example             Show an example configuration file
  --dump-config            Dump loaded configuration and exit
  --dump-schema            Dump loaded schema as JSON and exit (for debugging,
                           output structure is unstable)
  FILENAME                 Path to configuration file

When running postgrest-loadtest-against the tests run fine against the provided branch/tag but end with the above error against HEAD.

[taimoorzaeem]

@mkleczek Try removing the loadtest and result-bin directory every time before you run load tests.

[mkleczek]

@mkleczek Try removing the loadtest and result-bin directory every time before you run load tests.

I'm afraid it does not help.

Anyway - a workaround is to branch off main and use it as one of the test targets.

[steve-chavez]

Invalid argument `./result-bin/bin/postgrest'

Usage: postgrest [-v|--version] [-e|--example] [--dump-config | --dump-schema] 
                 [FILENAME]

  PostgREST 12.3 (pre-release)

@mkleczek The above is weird because it should PostgREST 13.1 as below:

$ postgrest-loadtest

postgrest-with-postgresql-17: You can connect with: psql 'postgres:///postgres?host=/run/user/1000/postgrest/postgrest-with-postgresql-17-8dK/socket' -U postgres
postgrest-with-postgresql-17: You can tail the logs with: tail -f /run/user/1000/postgrest/postgrest-with-postgresql-17-8dK/db.log
delaying data to/from postgres by 0ms
Building postgrest (nix)... 
done.
Starting PostgREST 13.1 (pre-release)... done.

I'm unable to reproduce.

Try removing the loadtest and result-bin directory every time before you run load tests.

Additionally I suggest to first commit all your changes and then do a git clean -xdf, just in case there are some leftovers (this has cleared errors for me before).

[mkleczek]

I've performed several rounds of tests and my conclusion is:

1. There is indeed a slight decrease in throughput compared to 13.0.4 but in my testing it is around 3%
2. Interestingly: the difference is larger with symmetric keys - not sure why.
3. The difference looks to be caused by metrics update. Once I removed the code that updates the metrics, HEAD became the same or slightly faster than 13.0.4 (but that could be just noise).

[steve-chavez]

The difference looks to be caused by metrics update.

Aha, that would make sense since it's a new addition to the JWT cache.

Once I removed the code that updates the metrics, HEAD became the same or slightly faster than 13.0.4 (but that could be just noise).

Would like to try too, to remove the metrics would it be enough to change this line:

postgrest/src/PostgREST/AppState.hs

Line 152 in fc1a5ec

<*> JwtCache.init conf observer

To this?

    <*> JwtCache.init conf (\_ -> pure ())

[mkleczek]

The difference looks to be caused by metrics update.

Aha, that would make sense since it's a new addition to the JWT cache.

Once I removed the code that updates the metrics, HEAD became the same or slightly faster than 13.0.4 (but that could be just noise).

Would like to try too, to remove the metrics would it be enough to change this line:

postgrest/src/PostgREST/AppState.hs

Line 152 in fc1a5ec

<*> JwtCache.init conf observer
To this?

<*> JwtCache.init conf (\_ -> pure ())

Yeah... That should work, although there would still be unnecessary creation of observation events:

    notCachingErrors :: STM Int -> JwkSet -> IO (SC.Cache (ExceptT Error IO) ByteString JSON.Object)
    notCachingErrors maxSize key = SC.cacheIO (SC.CacheConfig maxSize
            (parseAndDecodeClaims key)
            --------
            -- lines below create unnecessary observation events
            --------
            (lift . observationHandler . JwtCacheLookup) -- lookup metrics
            (const . const $ lift $ observationHandler JwtCacheEviction) -- evictions metrics
            alwaysValid) -- no invalidation for now