Uploaded image for project: 'Confluence Data Center'
  1. Confluence Data Center
  2. CONFSERVER-20475

Improve performance of DefaultSpacePermissionManager#hasPermissionViaGroups by reversing the way permissions are checked.

XMLWordPrintable

    • We collect Confluence feedback from various sources, and we evaluate what we've collected when planning our product roadmap. To understand how this piece of feedback will be reviewed, see our Implementation of New Features Policy.

      When Confluence checks permissions at the group level, it retrieves all the groups a user is a member of, then looks for a matching permission against each of those groups. This performs terribly when the user is a member of a large number of groups.

      Performance would almost always be better, or at least be much more predictable if you grab all the group-level permissions for the space, then perform memberOf() checks against the user for each group. (Especially in situations like LDAP where memberOf() is really cheap, but getting all groups for a user is painfully expensive)

            [CONFSERVER-20475] Improve performance of DefaultSpacePermissionManager#hasPermissionViaGroups by reversing the way permissions are checked.

            m@ (Inactive) added a comment - - edited

            Duplicated here: https://studio.atlassian.com/browse/EMBCWD-558

            m@ (Inactive) added a comment - - edited Duplicated here: https://studio.atlassian.com/browse/EMBCWD-558

            CharlesA added a comment -

            I wrote this patch today, but looking at it, the performance implications are really murky.

            • While multiple calls to hasMembership(user, group) could be made faster than one to getGroups(user).contains() (and ideally should), chances are it isn't right now.
            • The new approach bypasses the cache we put around findPermission(). Once again, there's no guarantee that we'll be faster on a warm cache
            • The new calls to getPermissions(type) might cause Hibernate to flush() permission data out of the session mid-check, further changing the performance profile of the permission manager.

            I wouldn't be comfortable committing this into Confluence without a significant amount of testing of permission performance under different configurations of users, groups and permissions.

            CharlesA added a comment - I wrote this patch today, but looking at it, the performance implications are really murky. While multiple calls to hasMembership(user, group) could be made faster than one to getGroups(user).contains() (and ideally should), chances are it isn't right now. The new approach bypasses the cache we put around findPermission(). Once again, there's no guarantee that we'll be faster on a warm cache The new calls to getPermissions(type) might cause Hibernate to flush() permission data out of the session mid-check, further changing the performance profile of the permission manager. I wouldn't be comfortable committing this into Confluence without a significant amount of testing of permission performance under different configurations of users, groups and permissions.

              matt@atlassian.com Matt Ryall
              cmiller CharlesA
              Votes:
              7 Vote for this issue
              Watchers:
              10 Start watching this issue

                Created:
                Updated:
                Resolved: