-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Adds ACL replication. #2237
Adds ACL replication. #2237
Conversation
This isn't ready for review but I'm pushing this up as a checkpoint as the basic plumbing is working. |
3caa4c8
to
f0a04bc
Compare
f0a04bc
to
096d5ff
Compare
Decided we don't need to log anything about the token here. If the token is not valid then the client will get an error about that, so anything that can happen here is related to talking to the server in the ACL datacenter, so not specific to the token.
Ready for review! |
We don't want ACL replication to have this behavior so it was a little dangerous to have in the shared helper function.
@@ -37,24 +37,29 @@ const ( | |||
redactedToken = "<hidden>" | |||
|
|||
// Maximum number of cached ACL entries | |||
aclCacheSize = 256 | |||
aclCacheSize = 10 * 1024 |
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.
Can we use this chance to switch from the LRU cache to 2Q? It's in the same package!
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.
Ah good call - I'll roll that in.
This fixes #1419 by providing a full replication capability for ACLs. Non-ACL datacenters can replicate the complete ACL set locally to their state store and fall back to that if there's an outage, if so configured. Additionally, this provides a good way to make a backup ACL datacenter, or to migrate the ACL datacenter to a different one. This'll fix #1826 and should also mention #1186.