Don't attempt to read public keys more than once every 5 minutes

[DrDaveD]

I haven't tested it, but according to the code it appears that if reading of the public keys fail, this library will re-try reading those keys with every validation attempt. Instead, there should be a "negative cache" recorded so the keys are only attempted to be read every 5 minutes. Otherwise it could end up with a much higher load on the server when it is already having problems, plus it could cause unnecessary delays on validation.

(As a side note, reading public keys every 10 minutes after a success seems excessive. 30 minutes sounds more reasonable to me. I would still leave re-tries every 5 minutes though. These numbers are based on my experience with cvmfs and frontier caching. The scitokens python library sets it to 60 minutes, which is also reasonable.)

[jbasney]

Agreed. CILogon will block or rate limit excessive requests to our endpoints, including our public key endpoint.

https://github.com/WLCG-AuthZ-WG/common-jwt-profile/blob/master/profile.md#token-lifetime-guidance sets a minimum lifetime of 1 hour for the public key cache. I think that's why the python library sets it to 60 minutes.

[DrDaveD]

This has become a production problem now. Fermilab worker nodes are being blocked by CILogon rate limits, and the problem gets multiplied once there's a failure because every invocation asks again. The problem is worse on IPv6 because AWS aggregates rate limits to entire /64 address ranges, so a whole bunch of worker nodes are being blocked together.

[djw8605]

I curious, is this being invoked by a single process, or multiple processes? Just thinking if this will have to be another table in the sqlite db. Do you workers have access to the sqlite db? I want to make sure if I come up with a solution that uses the sqlite db as the backend, it will work for you.

[DrDaveD]

There are multiple processes by the same user id on the node I am examining. I looked in the user's .cache/scitokensscitokens_cpp.sqllite with sqlite3 and select * from keycache and saw that during the rate-limited condition the next_update field was not advancing. I think all that's needed is to go ahead and add 5 minutes to that value if the lookup fails (besides increasing the default value after success from 10 minutes to 60 minutes).

[DrDaveD]

It doesn't really have to be 5 minutes on an error, it should just be at least 5 minutes. If you'd prefer to add 60 minutes whether it failed or succeeded that would be fine too.

[maarten-litmaath]

I think we should go to 60 minutes already to try and avoid further such issues altogether.

[DrDaveD]

Also FYI HTCondor is setting up a separate home directory per job so there ends up being multiple caches per user id. That should be fine; the problem was caused by frequent calls within each job. However I'm not actually sure if any jobs are doing parallel processes on the same cache; most likely the processes I saw were using different caches.

Meanwhile we are attacking this problem in several other ways as well, so we don't need to wait for the whole release and distribution process of scitokens-cpp, but this should be a priority to get fixed.