Description of the latest improvement in ACL checks

Hey, guys.

I’ve committed quite major change to ACL check that:

  1. split queries, but makes them more effective
  2. adds additional caching of positive results
  3. get rid of one join in a query
  1. about splitting queries
    If you’ll compare explains of the two queries below:
    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 )
    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:
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.

  1. 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.