RT 3.4 / Postgres / Search UI performance

I’ve spent the evening doing some stress testing / performance work on
RT 3.4’s search page under postgres. I have a proposed patch, but I
really need folks to beat on it to see if it improves things
significantly and whether it seems to actually do the right thing.

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

=== html/Elements/SelectOwner
— html/Elements/SelectOwner (revision 2678)
+++ html/Elements/SelectOwner (local)
@@ -67,25 +67,19 @@
@objects = keys %{$cfqueues};
}
else {

  •    my $Queues = RT::Queues->new($session{CurrentUser});
    
  • $Queues->UnLimit();
  • while (my $Queue = $Queues->Next()) {
  •     push( @objects, $Queue );
    
  •    }
    
  • Let’s check rights on an empty queue object. that will do a search for any queue.

  • my $queue = RT::Queue->new($session{‘CurrentUser’});
  •     push( @objects, $queue );
    

}

my %user_uniq_hash;
foreach my $object (@objects) {
my $Users = RT::Users->new($session{CurrentUser});

  •    $Users->WhoHaveRight(Right => 'OwnTicket',
    
  •                 Object => $object,
    
  •                 IncludeSystemRights => 1,
    
  •                 IncludeSuperusers => 0);
    
  •    while (my $User = $Users->Next()) {
    
  •    $Users->WhoHaveRight(Right => 'OwnTicket', Object => $object, IncludeSystemRights => 1, IncludeSuperusers => 0); while (my $User = $Users->Next()) {
        $user_uniq_hash{$User->Id()} = $User;
       }
    

}
-@users = sort { uc($a->Name) cmp uc($b->Name) } values %user_uniq_hash;
+@users = sort { uc($a->Name) cmp uc($b->Name) } values %user_uniq_hash;
</%INIT>

<%ARGS>
=== lib/RT/Users_Overlay.pm
— lib/RT/Users_Overlay.pm (revision 2678)
+++ lib/RT/Users_Overlay.pm (local)
@@ -239,6 +239,7 @@
or as members of groups

+If passed a queue object, with no id, it will find users who have that right for any queue

@@ -246,38 +247,121 @@

sub WhoHaveRight {
my $self = shift;

  • my %args = ( Right => undef,
  •             Object                 => undef,
    
  •             IncludeSystemRights    => undef,
    
  •             IncludeSuperusers      => undef,
    
  •             IncludeSubgroupMembers => 1,
    
  •             @_ );
    
  • my %args = (
  •    Right                  => undef,
    
  •    Object                 => undef,
    
  •    IncludeSystemRights    => undef,
    
  •    IncludeSuperusers      => undef,
    
  •    IncludeSubgroupMembers => 1,
    
  •    @_
    
  • );
  • if (defined $args{‘ObjectType’} || defined $args{‘ObjectId’}) {
  •    $RT::Logger->crit("$self WhoHaveRight called with the Obsolete ObjectId/ObjectType API");
    
  •    return(undef);
    
  • if ( defined $args{‘ObjectType’} || defined $args{‘ObjectId’} ) {
  •    $RT::Logger->crit( "$self WhoHaveRight called with the Obsolete ObjectId/ObjectType API");
    
  •    return (undef);
    
    }
  •    my @privgroups;
    
  •    my $Groups = RT::Groups->new($RT::SystemUser);
    
  •    $Groups->WithRight(Right=> $args{'Right'},
    
  •                 Object => $args{'Object'},
    
  •                 IncludeSystemRights => $args{'IncludeSystemRights'},
    
  •                 IncludeSuperusers => $args{'IncludeSuperusers'});
    
  •    while (my $Group = $Groups->Next()) {
    
  •            push @privgroups, $Group->Id();
    
  •    }
    
  • Find only members of groups that have the right.

  • if (@privgroups) {
  •    $self->WhoBelongToGroups(Groups => \@privgroups,
    
  •                             IncludeSubgroupMembers => $args{'IncludeSubgroupMembers'});
    
  • my $acl = $self->NewAlias(‘ACL’);
  • my $groups = $self->NewAlias(‘Groups’);
  • my $userprinc = $self->{‘princalias’};
    +# The cachedgroupmembers table is used for unrolling group memberships to allow fast lookups
    +# if we bind to CachedGroupMembers, we’ll find all members of groups recursively.
    +# if we don’t we’ll find only ‘direct’ members of the group in question
  • my $cgm;
  • if ( $args{‘IncludeSubgroupMembers’} ) {
  •    $cgm = $self->NewAlias('CachedGroupMembers');
    
    }
    else {
  • We don’t have any group that matches – make it impossible.

  • $self->Limit( FIELD => ‘Id’, VALUE => ‘IS’, OPERATOR => ‘NULL’ );
  •    $cgm = $self->NewAlias('GroupMembers');
    
    }
    +#Tie the users we’re returning ($userprinc) to the groups that have rights granted to them ($groupprinc)
  • $self->Join(
  •    ALIAS1 => $cgm,
    
  •    FIELD1 => 'MemberId',
    
  •    ALIAS2 => $userprinc,
    
  •    FIELD2 => 'id'
    
  • );
  • $self->Join(
  •    ALIAS1 => $groups,
    
  •    FIELD1 => 'id',
    
  •    ALIAS2 => $cgm,
    
  •    FIELD2 => 'GroupId'
    
  • );
    +# {{{ Find only rows where the right granted is the one we’re looking up or possibly superuser
  • $self->Limit(
  •    ALIAS    => $acl,
    
  •    FIELD    => 'RightName',
    
  •    OPERATOR => ( $args{Right} ? '=' : 'IS NOT' ),
    
  •    VALUE => $args{Right} || 'NULL',
    
  •    ENTRYAGGREGATOR => 'OR'
    
  • );
  • if ( $args{‘IncludeSuperusers’} and $args{‘Right’} ) {
  •    $self->Limit(
    
  •        ALIAS           => $acl,
    
  •        FIELD           => 'RightName',
    
  •        OPERATOR        => '=',
    
  •        VALUE           => 'SuperUser',
    
  •        ENTRYAGGREGATOR => 'OR'
    
  •    );
    
  • }
  • }}}

  • my ( $or_check_ticket_roles, $or_check_roles );
  • my $which_object = “$acl.ObjectType = ‘RT::System’”;
  • if ( defined $args{‘Object’} ) {
  •    if ( ref( $args{'Object'} ) eq 'RT::Ticket' ) {
    
  •        $or_check_ticket_roles = " OR ( $groups.Domain = 'RT::Ticket-Role' AND $groups.Instance = " . $args{'Object'}->Id . ") ";
    

+# If we’re looking at ticket rights, we also want to look at the associated queue rights.
+# this is a little bit hacky, but basically, now that we’ve done the ticket roles magic,
+# we load the queue object and ask all the rest of our questions about the queue.

  •        $args{'Object'} = $args{'Object'}->QueueObj;
    
  •    }
    
  •    # TODO XXX This really wants some refactoring
    
  •    if ( ref( $args{'Object'} ) eq 'RT::Queue' ) {
    
  •        $or_check_roles = " OR ( ( ($groups.Domain = 'RT::Queue-Role' ";
    
  •        $or_check_roles .= "AND $groups.Instance = " . $args{'Object'}->id if ( $args{'Object'}->id );
    
  •        $or_check_roles .= ") $or_check_ticket_roles ) " . " AND $groups.Type = $acl.PrincipalType) ";
    
  •    }
    
  •    if ( $args{'IncludeSystemRights'} ) {
    
  •        $which_object .= ' OR ';
    
  •    }
    
  •    else {
    
  •        $which_object = '';
    
  •    }
    
  •    $which_object .= " ($acl.ObjectType = '" . ref( $args{'Object'} ) . "'";
    
  •    if ( $args{'Object'}->id ) {
    
  •        $which_object .= " AND $acl.ObjectId = " . $args{'Object'}->id;
    
  •    }
    
  •    $which_object .=  ") ";
    
  • }
  • $self->_AddSubClause( “WhichObject”, “($which_object)” );
  • $self->_AddSubClause(
  •    "WhichGroup",
    
  •        qq{ ( (    $acl.PrincipalId = $groups.id AND $acl.PrincipalType = 'Group' 
    
  •            AND (   $groups.Domain = 'SystemInternal' OR $groups.Domain = 'UserDefined' OR $groups.Domain = 'ACLEquivalence')) 
    
  •            $or_check_roles) }
    
  • );
  • only include regular RT users

  • $self->LimitToEnabled;
  • no system user

  • $self->Limit( ALIAS => $userprinc, FIELD => ‘id’, OPERATOR => ‘!=’, VALUE => $RT::SystemUser->id);
    }

}}}

{{{ WhoBelongToGroups

@@ -310,20 +394,14 @@
$cgm = $self->NewAlias(‘GroupMembers’);
}

  • {{{ Tie the users we’re returning ($userprinc) to the groups that have rights granted to them ($groupprinc)

  • #Tie the users we’re returning ($userprinc) to the groups that have rights granted to them ($groupprinc)
    $self->Join( ALIAS1 => $cgm, FIELD1 => ‘MemberId’,
    ALIAS2 => $userprinc, FIELD2 => ‘id’ );
  • }}}

  • my $and_check_groups = “($cgm.GroupId = NULL”;

    foreach my $groupid (@{$args{‘Groups’}}) {
    $self->Limit(ALIAS => $cgm, FIELD => ‘GroupId’, VALUE => $groupid, QUOTEVALUE => 0, ENTRYAGGREGATOR=> ‘OR’)

  •    #$and_check_groups .= " OR $cgm.GroupId = $groupid";
    

    }

  • #$and_check_groups .= “)”;

  • #$self->_AddSubClause(“WhichGroup”, $and_check_groups);
    }

}}}

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

I will make sure we get this tested, but it’ll take a few days.

A

Andrew Sullivan | ajs@crankycanuck.ca
I remember when computers were frustrating because they did exactly what
you told them to. That actually seems sort of quaint now.
–J.D. Baldwin

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

I will make sure we get this tested, but it’ll take a few days.

nod I spent some time before dinner prodding at things and it seemed
like 90% of the time was being spent in the selectowner widget. It was
doing this loop:

For each queue:
Find each group that can own tickets in that queue.
For each group:
find its members.

That’s what we get for using a nice generalized layered API.

The new call builds a single query that say:

Find all the people who are members of groups that can own tickets an any queue.

It’s a bit hacky as yet, but hopefully it should work right.

I’ve spent the evening doing some stress testing / performance work on
RT 3.4’s search page under postgres. I have a proposed patch, but I
really need folks to beat on it to see if it improves things
significantly and whether it seems to actually do the right thing.

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

Wow… huge improvement here. 20+ seconds to bring up this
one page is now down to around 2 seconds.

This patch gets my vote of approval :slight_smile:

For what it’s worth, we have 18 queues defined, which explains why
the situation was so bad for us.

David Kerry

I’ve spent the evening doing some stress testing / performance work on
RT 3.4’s search page under postgres. I have a proposed patch, but I
really need folks to beat on it to see if it improves things
significantly and whether it seems to actually do the right thing.

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

Wow… huge improvement here. 20+ seconds to bring up this
one page is now down to around 2 seconds.

Is the list of possible owners correct?

J

I’ve spent the evening doing some stress testing / performance work on
RT 3.4’s search page under postgres. I have a proposed patch, but I
really need folks to beat on it to see if it improves things
significantly and whether it seems to actually do the right thing.

It’s attached to this message. If folks see a speed boost without
errors, it’ll make it into 3.4.0. But this means taht I need sites
running postgres (and other databases) to test it out. It’s likey that
the issue in question only manifests with a large number of queues.

Wow… huge improvement here. 20+ seconds to bring up this
one page is now down to around 2 seconds.

Is the list of possible owners correct?

J

It appears to be correct.

David Kerry