Re: [dev] Proposal for speeding up TicketAcls
Hi Moritz,
((enjoy))
Carlos Rodríguez
On Dec 12, 2013, at 11:12 AM, Moritz Lenz moritz.l...@noris.de wrote:
Hi,
On 12/12/2013 09:18 AM, Martin Gruner wrote:
Hi Moritz,
this sounds great to me.
and it turns out it's not too hard to implement either :-)
See https://github.com/OTRS/otrs/pull/180
Am 10.12.13 13:58, schrieb Moritz Lenz:
Hi all,
recently I found out that although we use only very few ACLs, the ACL
mechanism slows down our OTRS installation. Part of the problem is that
our CustomerUser-backend talks to an external data source, which is
slow. But the problem is more general: Lots of data is assembled in
Kernel::System::Ticket-TicketACL, which isn't actually used later on.
Since ACLs are also checked for ticket lists (like in AgentTicketQueue
or AgentTicketSearch), this can be a significant overhead.
For normal ACLs, we can easily find out which data they actually use.
For ACL modules, I propose we add extra configuration.
Before, the configuration of an ACL module looks like this in
Kernel::Config:
$Self-{'Ticket::Acl::Module'}-{'TheAclModulename'} = {
Module = 'Kernel::System::Ticket::Acl::TheAclModule',
};
If we extend that to something like this:
$Self-{'Ticket::Acl::Module'}-{'TheAclModulename'} = {
Module = 'Kernel::System::Ticket::Acl::TheAclModule',
Checks = ['Owner', 'Queue', 'SLA', 'Ticket'],
ReturnType = 'Ticket',
ReturnSubType = ['State', 'Service'],
};
we can apply two optimizations:
1) Only invoke an Acl Module if the ReturnType and ReturnSubType
overlap with the TicketAcl params ReturnType and ReturnSubType
2) Only gather data for the requested Checks elements. If for example no
Acl and no ACL module depends on the Service or the CustomerUser, than
those pieces of infomation don't need to retrieved at all.
Yes. This can be implemented backwards compatible, i. e. if the
configuration is not present, always run the ACL.
That's how I've done it in
https://github.com/moritz/otrs/commit/e1ccd3f5d8b625580e52c41d21d8846062f9f32f
As a further note, most information seems to show up twice in the
%Checks argument to the Acl module, for example both as $Checks{Owner}
and $Checks{Ticket}{Owner}. To get the desired speedup, a module which
only lists the check 'Ticket' would get whatever data TicketGet returns,
not the additional data that is currently added in method TicketAcl.
I'm not sure about this one. Let's hear what Carlos Rodriguez has to
say about it.
I have mitigated possible fallout from this approach by looking a bit deeper
into the ACL. If for example an ACL has
Properties = {
Ticket = {
Queue = ...
}
}
then both the Ticket and the Queue data is fetched. Which makes all the
available ACL unit tests pass.
The problem I see here is that Properties hash does not mandatory needs a
Ticket hash, but it could be for example:
Properties = {
Queue = {
Calendar = [‘MyCalendar’],
},
},
Then if we get the parameter ‘QueueID’ we needed also to do a QueueGet() in
this case (in order to match the Calendar name with the ACL).
P.S. for the sake of completeness I've gone through Kernel::System::Ticket,
and looked up the possible values for Checks, ReturnType and ReturnSubType.
Here they are:
Checks:
CustomerUser
DynamicField
Frontend
Owner
Priority
Process
Queue
Responsible
Service
SLA
State
Ticket
Type
ReturnType:
Action
Ticket
ReturnSubType:
Type
Queue
State
Service
SLA
Priority
Nice. Please feel free to add this to the POD, and also to improve the
existing code where you see fit.
I'll update the documentation a bit.
Cheers,
Moritz
___
OTRS mailing list: dev - Webpage: http://otrs.org/
Archive: http://lists.otrs.org/pipermail/dev
To unsubscribe: http://lists.otrs.org/cgi-bin/listinfo/dev
signature.asc
Description: Message signed with OpenPGP using GPGMail
___
OTRS mailing list: dev - Webpage: http://otrs.org/
Archive: http://lists.otrs.org/pipermail/dev
To unsubscribe: http://lists.otrs.org/cgi-bin/listinfo/dev
Re: [dev] Proposal for speeding up TicketAcls
Hi Carlos,
On 12/14/2013 01:45 AM, Carlos Rodríguez wrote:
On Dec 12, 2013, at 11:12 AM, Moritz Lenz moritz.l...@noris.de
mailto:moritz.l...@noris.de wrote:
Hi,
On 12/12/2013 09:18 AM, Martin Gruner wrote:
Hi Moritz,
this sounds great to me.
and it turns out it's not too hard to implement either :-)
Seehttps://github.com/OTRS/otrs/pull/180
Am 10.12.13 13:58, schrieb Moritz Lenz:
As a further note, most information seems to show up twice in the
%Checks argument to the Acl module, for example both as $Checks{Owner}
and $Checks{Ticket}{Owner}. To get the desired speedup, a module which
only lists the check 'Ticket' would get whatever data TicketGet returns,
not the additional data that is currently added in method TicketAcl.
I'm not sure about this one. Let's hear what Carlos Rodriguez has to
say about it.
I have mitigated possible fallout from this approach by looking a bit
deeper into the ACL. If for example an ACL has
Properties = {
Ticket = {
Queue = ...
}
}
then both the Ticket and the Queue data is fetched. Which makes all
the available ACL unit tests pass.
The problem I see here is that Properties hash does not mandatory needs
a Ticket hash, but it could be for example:
Properties = {
Queue = {
Calendar = [‘MyCalendar’],
},
},
Then if we get the parameter ‘QueueID’ we needed also to do a QueueGet()
in this case (in order to match the Calendar name with the ACL).
I've stumbled onto this problem during the implementation, and the
solution is simple: TicketAcl also calls TicketGet and pre-fills the
Ticket hash, because pretty much everything else depends on it.
There are lots of unit tests like this, for example
https://github.com/moritz/otrs/blob/feature-ticketacl-master/scripts/test/Ticket/TicketACL.t#L363
and the branch doesn't regress on any tests (and adds a few, fwiw).
Cheers,
Moritz
___
OTRS mailing list: dev - Webpage: http://otrs.org/
Archive: http://lists.otrs.org/pipermail/dev
To unsubscribe: http://lists.otrs.org/cgi-bin/listinfo/dev
Re: [dev] Proposal for speeding up TicketAcls
Hi,
On 12/12/2013 09:18 AM, Martin Gruner wrote:
Hi Moritz,
this sounds great to me.
and it turns out it's not too hard to implement either :-)
See https://github.com/OTRS/otrs/pull/180
Am 10.12.13 13:58, schrieb Moritz Lenz:
Hi all,
recently I found out that although we use only very few ACLs, the ACL
mechanism slows down our OTRS installation. Part of the problem is that
our CustomerUser-backend talks to an external data source, which is
slow. But the problem is more general: Lots of data is assembled in
Kernel::System::Ticket-TicketACL, which isn't actually used later on.
Since ACLs are also checked for ticket lists (like in AgentTicketQueue
or AgentTicketSearch), this can be a significant overhead.
For normal ACLs, we can easily find out which data they actually use.
For ACL modules, I propose we add extra configuration.
Before, the configuration of an ACL module looks like this in
Kernel::Config:
$Self-{'Ticket::Acl::Module'}-{'TheAclModulename'} = {
Module = 'Kernel::System::Ticket::Acl::TheAclModule',
};
If we extend that to something like this:
$Self-{'Ticket::Acl::Module'}-{'TheAclModulename'} = {
Module = 'Kernel::System::Ticket::Acl::TheAclModule',
Checks = ['Owner', 'Queue', 'SLA', 'Ticket'],
ReturnType = 'Ticket',
ReturnSubType = ['State', 'Service'],
};
we can apply two optimizations:
1) Only invoke an Acl Module if the ReturnType and ReturnSubType
overlap with the TicketAcl params ReturnType and ReturnSubType
2) Only gather data for the requested Checks elements. If for example no
Acl and no ACL module depends on the Service or the CustomerUser, than
those pieces of infomation don't need to retrieved at all.
Yes. This can be implemented backwards compatible, i. e. if the
configuration is not present, always run the ACL.
That's how I've done it in
https://github.com/moritz/otrs/commit/e1ccd3f5d8b625580e52c41d21d8846062f9f32f
As a further note, most information seems to show up twice in the
%Checks argument to the Acl module, for example both as $Checks{Owner}
and $Checks{Ticket}{Owner}. To get the desired speedup, a module which
only lists the check 'Ticket' would get whatever data TicketGet returns,
not the additional data that is currently added in method TicketAcl.
I'm not sure about this one. Let's hear what Carlos Rodriguez has to
say about it.
I have mitigated possible fallout from this approach by looking a bit
deeper into the ACL. If for example an ACL has
Properties = {
Ticket = {
Queue = ...
}
}
then both the Ticket and the Queue data is fetched. Which makes all the
available ACL unit tests pass.
P.S. for the sake of completeness I've gone through Kernel::System::Ticket,
and looked up the possible values for Checks, ReturnType and ReturnSubType.
Here they are:
Checks:
CustomerUser
DynamicField
Frontend
Owner
Priority
Process
Queue
Responsible
Service
SLA
State
Ticket
Type
ReturnType:
Action
Ticket
ReturnSubType:
Type
Queue
State
Service
SLA
Priority
Nice. Please feel free to add this to the POD, and also to improve the
existing code where you see fit.
I'll update the documentation a bit.
Cheers,
Moritz
___
OTRS mailing list: dev - Webpage: http://otrs.org/
Archive: http://lists.otrs.org/pipermail/dev
To unsubscribe: http://lists.otrs.org/cgi-bin/listinfo/dev
