fix: cpu contention when reading JWKs and suppress generating duplicate JWKs

[terev]

Previously each concurrent caller would need to lock a shared mutex when reading or writing a given JWK set. The read path now doesn't require locking a mutex at all and instead returns valid query results directly. The write path is now protected by a concurrency control mechanism (using x/sync/singleflight) to ensure only one JWK set is generated and persisted.

Note: Duplicate JWK sets may still be improperly generated if running more than one Hydra instance in a high traffic environment.

Related issue(s)

Fixes #3863

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  security@ory.sh) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

This fixes a bug #3876 with the previous attempt to fix this issue .

[aeneasr]

What would be the correct way to verify that this change is indeed behaving as expected? I assume we need some type of race test that calls the helper function multiple times and it has to give one expected result?

I think it would be good to have a test for this, as otherwise it's unclear if this fix works or breaks like the last time we merged a fix for this!

Thank you :)

[terev]

@aeneasr Yeah that's a good idea.

I think a good test would be to invoke the function with parallel calls that include a change to every argument that should cause unique results. Then ensure that each parallel caller gets a different result.

I can give this a shot if you think that sounds reasonable.

[aeneasr]

@aeneasr Yeah that's a good idea.

I think a good test would be to invoke the function with parallel calls that include a change to every argument that should cause unique results. Then ensure that each parallel caller gets a different result.

I can give this a shot if you think that sounds reasonable.

Nice! So I think a test set up like this would work:

1. Prepare a list of expected keys to be generated (more than 5)
2. For each key execute the operation (more than 5)
3. Now you have 5x5 combinations, run all of them at the same time in an errgroup
4. No race conditions expected, and 5 unique values at the end

[aeneasr]

The numbers probably depend, it maybe enough to generate 2 unique keys and call it 25 times instead. I think the big challenge here is making sure that a lot of requests on this function do not cause problems.

I'm not sure if we depend on database magic for locking, if we do, you may need to run this against a real database because SQLite does not support concurrent writes iirc

[aeneasr]

Would you still be open for the change?

[terev]

Yeah I mean to take a look, probably next weekend if possible.