search

scitokens / scitokens-cpp

A C++ implementation of the SciTokens library with a C library interface
Apache License 2.0
5 stars 22 forks source link

Add mutex around key refresh with get_public_keys_from_web() #137

Closed jthiltges closed 3 months ago

jthiltges commented 4 months ago

Limit key refresh to a single simultaneous request to avoid overloading issuers.

jthiltges commented 4 months ago

As far as I can tell, this will only limit downloads when we already have a valid token in the database. We will still have a stampede when we first see an issuer, but hopefully not after that?

That's correct. This PR focused on the refresh case alone, since that was causing the issue at hand. I could work on expanding it to include new issuers if you prefer.

djw8605 commented 4 months ago

I'm wondering what type of unit tests would test this? Though, I don't think we need a unittest for this.

djw8605 commented 4 months ago

Have you tested this on a system?

jthiltges commented 4 months ago

I've tested it lightly with a build that included some logging to stdout, and it seemed to work as intended: for a burst of requests and an expired key, saw a single attempt on the issuer. I'd also checked with packet capture, and confirming just two connections to the issuer.

Getting proper testing would be great. Should be do-able, but I think we'd need some sort of logging from the library to verify.

djw8605 commented 4 months ago

I would accept just making a quick build and running it on a production xrootd server for ~48 hours.

jthiltges commented 3 months ago

Getting back to this, I'd made a scratch build and have been running it on Nebraska xfer servers--along with cmsaf-dev xcaches--for a number of days now. It doesn't appear to have caused any issues.