Skip to content

perf(acl): key AclCache.userPredPerms by namespace-qualified user ID #9861

Open

Description

@matthewmcneely

Summary

AclCache.userPredPerms is keyed by bare user ID, while every predicate key inside it is namespace-qualified (x.NamespaceAttr(ns, pred)). User IDs are namespace-local, so the same ID in two namespaces lands on one shared entry.

#9812 fixes the correctness bug this caused (refreshing one namespace replaced the user's permissions from every other namespace) by merging per namespace instead of replacing. That's the right fix for now, but the shared key is still the root cause, and it carries two costs that #9812 makes slightly worse:

  • Refresh cost. Update clears the refreshed namespace by walking every user's full predicate map, and RefreshACLs calls Update once per namespace, so a full refresh is O(namespaces × total entries).
  • Per-query cost. authorizePreds iterates the user's predicates across all namespaces to build allowedPreds, and expand(_all_) builds a hash map over that slice on every query. For a user ID present in many namespaces (common in multi-tenant setups) that grows linearly with namespace count. Since fix(acl): preserve user permissions across namespaces #9812, GetUserPredPerms also copies the map on every call.

Proposal

Key the outer map by x.NamespaceAttr(ns, userID) and change the getter to GetUserPredPerms(ns, userId). authorizePreds already has ns := userData.namespace in hand, so the caller change should be contained.

With that in place:

  • Clearing a namespace in Update only has to check the outer keys and drop whole entries, so it's O(users) instead of O(total entries).
  • The merge loop and empty-entry pruning added in fix(acl): preserve user permissions across namespaces #9812 go away. Update can replace its namespace's entries outright again.
  • authorizePreds, expand(_all_), and the snapshot copy only see the caller's namespace.

Notes

Follow-up from the review on #9812: #9812 (comment)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions