Modify

Opened 3 years ago

Last modified 11 months ago

#9138 assigned enhancement

[Patch] PrivateTicketsPlugin > LDAP Slowness

Reported by: bobbysmith007 Owned by: rjollos
Priority: normal Component: PrivateTicketsPlugin
Severity: normal Keywords: ldap performance slowness
Cc: Trac Release: 0.11

Description

When LDAP is the permissions store the _groups function takes a long time to complete.

I apparently included some of your code in my plugin a while ago and had a forever-open-ticket to resolve the slow LDAP issues, #3828.

Today I implemented the most basic possible fix for this in [10626]. As best I can tell this should mostly resolve the issue (by not querying all permissions from LDAP). My internal tickets are still internal after this as well.

I'm not sure what other IPermissionStores there are, so not sure how to test further.

Hope this helps some of your users

Attachments (0)

Change History (11)

comment:1 Changed 3 years ago by bobbysmith007

ps the issue arises because the default permission store does not include user created groups when asked for the groups.

comment:2 Changed 3 years ago by rjollos

  • Summary changed from PrivateTicketsPlugin > LDAP Slowness to [Patch] PrivateTicketsPlugin > LDAP Slowness

comment:3 Changed 3 years ago by rjollos

  • Status changed from new to assigned

I'm seeing an addition 4-5 seconds latency for /ticket and /newticket when the PrivateTicketsPlugin is enabled. I'm not using LDAP. I'll test out your fix and see if it addresses the issue.

comment:4 in reply to: ↑ description Changed 3 years ago by rjollos

Replying to bobbysmith007:

I'm not sure what other IPermissionStores there are, so not sure how to test further.

This is all new to me, but it looks like there are a couple of plugins that implement an alternate permission store: LdapPlugin, ActiveDirectoryAuthPlugin, SuperUserPlugin, TracForgePlugin. It looks like there can also only be one permission store per Trac project (see t:TracIni#trac-section and I also took a look at the Trac source to confirm this).

ps the issue arises because the default permission store does not include user created groups when asked for the groups.

Okay, I think that I understand. get_permission_groups only returns authenticated and anonymous when called for the DefaultPermissionGroupProvider.

Is the idea of your patch that, when the PermissionStore is LDAP, get_permission_groups will return the user-created groups, and therefore we don't need (or want) to query and search through the entire set of permissions to get the user-created groups? It sounds like your working assumption is that the only case we need to query and search all the permissions in order to find user-defined groups is in the case of a default permissions store, and provided this assumption holds for other permission stores, then the patch will be solid.

If my assumptions about your patch are true, and since I don't have the time or resources to setup and test against all the permissions stores, I'd be willing to run with your patch. I might post a note to the project wiki page saying that the PrivateTicketsPlugin has only been developed against the DefaultPermissionStore, and request feedback if anyone has tested it with a different permission store.

It is not immediately obvious to me why Trac wouldn't provide a method that returns all the user-defined groups that a particular subject is a member of, implementing something similar to this code:

perms = PermissionSystem(self.env).get_all_permissions()
repeat = True
while repeat:
    repeat = False
        for subject, action in perms:
            if subject in groups and not action.isupper() and action not in groups:
                groups.add(action)
                repeat = True 

comment:5 Changed 3 years ago by rjollos

Here is some similar and relevant code form the Trac source:

There has been some recent work on caching the permissions in t:#4245, which might help with the slowness issues we are experiencing. To address my issue in comment:3, I'll probably need to start looking at how we can optimize the performance of the code in _get_groups.

comment:6 Changed 3 years ago by rjollos

  • Keywords ldap performance slowness added

comment:7 follow-up: Changed 3 years ago by bobbysmith007

It is not immediately obvious to me why Trac wouldn't provide a method that returns all the user-defined groups

I think there should be a method like this in trac, but I was unable to locate it, but was able to find what I needed in PrivateTicketsPlugin/policy.py. Then a bit later I made that code path only work for the default permission store, because LDAP was taking minutes to return results (apparently).

Perhaps a new ticket with this patchified. Until that time though, this solved the problem for me (who does not use LDAP for authorization).

It sounds like your working assumption is that the only case we need to query and search all the permissions in order to find user-defined groups is in the case of a default permissions store

Rather, I would say my assumption was that this code path was running afoul of very large permissions stores (eg: LDAP), but was known to work when using the DefaultPermission store. As I only needed to use the default and didn't have the time / inclination to test against all the plug-able ones, I simply made it only work for the default. (I probably should have included an note about this, as you propose.)

comment:8 in reply to: ↑ 7 Changed 3 years ago by rjollos

Replying to bobbysmith007:

Rather, I would say my assumption was that this code path was running afoul of very large permissions stores (eg: LDAP), but was known to work when using the DefaultPermission store.

It sounds like you are saying its likely that LDAP itself isn't necessarily the cause of the slowdown, rather it is related to the number of users and groups. I'm seeing an approximately 5 second increase in ticket page load times due to the PrivateTicketsPlugin, with approximately 50 users and 5 groups (and the time seems to be creeping up as I add additional users and groups, a while back it was a 4 second increase). It sounds like this code just needs some significant optimization, which may provide significant benefits for any permission store.

comment:9 Changed 3 years ago by bobbysmith007

It sounds like you are saying its likely that LDAP itself isn't necessarily the cause of the slowdown, rather it is related to the number of users and groups

That seems like it could be accurate, though yours is the first data I have on that aspect.

comment:10 Changed 18 months ago by rjollos

  • Status changed from assigned to new

comment:11 Changed 11 months ago by rjollos

  • Status changed from new to assigned

Add Comment

Modify Ticket

Action
as assigned The owner will remain rjollos.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.