-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use a role cache to avoid separate locking paths #6926
Conversation
Due to the various locked/nonlocked paths we had a case where we weren't always checking for secondary status before trying to upgrade. This broadly simplifies things by using a cache to store the current role values (avoiding a lot of storage hits) and updating the cache on any write, delete, or invalidation.
Co-Authored-By: Vishal Nayak <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me. The tests that have been added are passing. I do see a test failure in the aws credential plugin directory, but it appears unrelated:
--- FAIL: TestBackendAcc_LoginWithCallerIdentity (2.42s)
backend_test.go:1564: bad: failed to create role: resp:&logical.Response{Secret:<nil>, Auth:<nil>, Data:map[string]interface {}{"error":"unable to resolve ARN \"arn:aws:iam::047646243813:user/tbex\" to internal ID: InvalidClientTokenId: The security token included in the request is invalid\n\tstatus code: 403, request id: 46f88dbd-92de-11e9-ba31-51c4e4fb1518"}, Redirect:"", Warnings:[]string(nil), WrapInfo:(*wrapping.ResponseWrapInfo)(nil), Headers:map[string][]string(nil)}
err:<nil>
I haven't been able to verify yet if this failure also occurs on master because it's not part of automated tests.
Yeah that seems unrelated. |
This test passes for me:
|
* Use a role cache to avoid separate locking paths Due to the various locked/nonlocked paths we had a case where we weren't always checking for secondary status before trying to upgrade. This broadly simplifies things by using a cache to store the current role values (avoiding a lot of storage hits) and updating the cache on any write, delete, or invalidation.
Due to the various locked/nonlocked paths we had a case where we weren't
always checking for secondary status before trying to upgrade. This
broadly simplifies things by using a cache to store the current role
values (avoiding a lot of storage hits) and updating the cache on any
write, delete, or invalidation.