Hey, guys.
I’ve committed quite major change to ACL check Principals.pm that:
- split queries, but makes them more effective
- adds additional caching of positive results
- get rid of one join in a query
- about splitting queries
If you’ll compare explains of the two queries below:
SELECT id FROM grp WHERE
type = ‘Owner’
AND (
( domain = ‘RT::Ticket-Role’ AND instance = 10 )
OR ( domain = ‘RT::Queue-Role’ AND instance = 3 )
OR ( domain = ‘RT::System-Role’ AND instance = 1 )
);
SELECT STRAIGHT_JOIN g.id FROM acl a, grp g WHERE
g.type = a.type
AND (
( g.domain = ‘RT::Ticket-Role’ AND g.instance = 10 )
OR ( g.domain = ‘RT::Queue-Role’ AND g.instance = 3 )
OR ( g.domain = ‘RT::System-Role’ AND g.instance = 1 )
);
then you’ll see that in the first mysql uses “range” scan and in the
second one “ref”, usually ref is better than range, but if you’ll look
closer then you’ll see that ref scan uses only first part
(‘group.type’) part of an index. That’s bug of mysql and it’s been
fixed in 5.0.45 only. Why it’s so bad for RT? That’s bad as we have
one group per ticket and type combination, so if you have 10k tickets
then you have 10k groups of ‘Owner’ type. So for each record in ACL
table mysql finds 10k groups using index, for each group it fetches
other fields from the table data and applies other conditions. In
mysql 5.0.45 this bug has been fixed and mysql uses range scan and
gets much less records and filter them using an index only.
Now we split this query into group:
SELECT STRAIGHT_JOIN g.id FROM acl a, grp g WHERE
g.type = a.type
AND ( g.domain = ‘RT::Ticket-Role’ AND g.instance = 10 )
it’s even more effective as mysql and other DBs too can use ref access
type and get set of records using an index only even not applying
where conditions, however we have to run more queries in the worst
case so it should be quite balanced.
This check we do quite often and people should see improvement when
they grant many rights via roles.
Anyway there is less used query that get all users who has right two
own tickets in all queues, we run it when we build the query builder
and it’s not affected by this improvement. I think we should change
this as well, but I have no patch.
- about additional caching of positive results
I’ve added new type of caching of positive results what can reduce
number of request really dramatically, especially for privileged users
who really have rights. Before we very caching positive and negative
answers using uid, right and whole list of acl equiv objects. However
if user is member of a group that has the right on one of those
objects directly (not via roles) then really user has that right one
any object which list the same acl equiv object. For example if we
check a right on a ticket X then we list System, Queue-Y and Ticket-X.
Imagine that user has that right via group or personal on Queue-Y what
means that he really has the right to all tickets in Y. In this case
we also cache result with key: uid, right, object. Then we’ll fetch
results from cache for any request where queue Y is listed as ACL
equiv object. We don’t cache negative answers so the old cache is
still effective and left in its place.
Best regards, Ruslan.