Improve trusted cert loading in Certificate Auth #27902
Conversation
Currently, cert auth has a cache of certName->trusted certificate data. This cache is updated lazily on login. In highly concurrent situations, several logins of the same cert or more likely, logins not specifying role name may happen simulataneously. In the status quo, each results in going to storage, fetching the role data (or all roles!), unmarshalling, and certificate parsing. This change puts a lock matrix in front of the cache miss scenario, so only one of the logins will load and process the role data. In addition, we treat the absent role name specially, caching it separately so that it cannot be flushed by eviction on the role cache.
github-actions
bot
added
the
hashicorp-contributed-pr
|
CI Results: |
sgmiller
added
backport/ent/1.15.x+ent
backport/ent/1.16.x+ent
|
Build Results: |
| lock := locksutil.LockForKey(b.trustedCacheLocks, certName) | ||
| lock.Lock() | ||
| defer lock.Unlock() | ||
|
|
There was a problem hiding this comment.
To avoid re-doing the work, shouldn't we check to make sure that b.trustedCache.Get(certName) keep doing the work only if nil, otherwise leverage the newly populated cache while we were locked?
There was a problem hiding this comment.
You only get to this point after a cache test in getTrustedCerts, so yeah, there is a tiny race here where it could have been filled in prior to getting the lock. Worth another test?
There was a problem hiding this comment.
Well the point of the lock was to avoid different clients from re-populating the cache? All this will do is actually make it worse by serializing all the requests but they will still re-populate the cache no?
There was a problem hiding this comment.
Doh! You're right. I should take an RLock in the read case. And sure, I'll do a re-test.
Currently, cert auth has a cache of certName->trusted certificate data. This cache is updated lazily on login. In highly concurrent situations, several logins of the same cert or more likely, logins not specifying role name may happen simulataneously. In the status quo, each results in going to storage, fetching the role data (or all roles!), unmarshalling, and certificate parsing.
This change puts a lock matrix in front of the cache miss scenario, so only one of the logins will load and process the role data. In addition, we treat the absent role name specially, caching it separately so that it cannot be flushed by eviction on the role cache.