rendering the fund drop-down in the line batch update controls can be slow

Bug #1254918 reported by Galen Charlton
30
This bug affects 6 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
2.6
Fix Released
Medium
Unassigned

Bug Description

During the process of rendering the list of permitted funds in the batch change interface for the line item / purchase order tables, the following pcrud call gets issued:

open-ils.pcrud.search.acqf.atomic "SESSIONKEY",{"active":"t"},{}

For at least one consortium with over 1500 active funds, this has been observed to take in excess of 20 seconds, which means that bringing up a purchase order in the staff client, even one with few line items, can be slow. So far I assume that it's simply bogging down making 1,500+ calls to permission.usr_has_object_perm().

Evergreen 2.4

Galen Charlton (gmc)
tags: added: acq performance
Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

I've seen the slowness in the fund field in the batch updater since we upgraded to 2.4 as well as seeing it in the fund field on the Copies page since 2.0. In both places it takes about 5 seconds for the fund field to completely load. (We have just over 400 active funds currently.)

Ben Shum (bshum)
Changed in evergreen:
status: New → Confirmed
importance: Undecided → Medium
Revision history for this message
Mike Rylander (mrylander) wrote :

Part 1: Speed up PCRUD permission testing globally
------------------------------------------------------------------------

Previous to this commit, permissions in Evergreen check a combinbation of:
 * user-object permissions (does the user have a direct permission mapping to the object in question)
 * user-context permissions (does the user have the permission at the object's context location, whose field is defined in the IDL)
 * user-global permission (lacking a context location, does the user have the permission globally (at the top of the org tree) and therefore can apply the action to all objects of this typ)

In practice, there are almost no user-object permissions. When retrieving just on object from the database, the cost of this check is negligable to the point that we can completely ignore it. However, when retrieving a large set of objects, such as the list of all funds in a large, consortial environment, the cost to check the user-object permission adds up to a noticable amount of time.

To address this, we add a new construct to the IDL instructing the PCRUD infrastructure to skip user-object permission checking in those cases where the design and use of the system makes user-specific object permissions needless or superfluous. This is embodied in a new XML attribute on the <pcrud> element: ignore_object_perms. When set to "true", pcrud will skip all user-object permission checks, resulting in faster time-to-first-result.

Additionally, we add a new "owning_user" attribute on the <action> element of the <pcrud> section. This new attribute specifies the field containing the actor.usr.id of the user that "owns" the object. This allows PCRUD to test ownership of an object directly, and if the requesting user and owning user are the same, the action is allowed.

Finaly, when "global_required" is "true" for the permission check, and there is no "owning_user" attribute defined for the class in the IDL, we skip the above-mentioned user-object permission check. When "global_required" is "false" or there is an "owning_user" attribute, we check for user permissions.

In all cases, the "ignore_object_perms" attribute is honored, and in its presence we skip non-owner user-object permissions.

The net result is an immediate increase in speed for retrieval of objects in the presence of the "global_required" attribute, and a mechanism to increase the speed of specific cases of context-aware retrival by the use of "ignore_object_perms".

We use this new mechanism to speed the retrieval of fund objects in the ACQ interfaces that draw available-fund dropdowns.

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/collab/miker/lp1254918_avoid-user-perm-checks

Revision history for this message
Bill Erickson (berick) wrote :

Part 2: Avoid unneeded permission checks in fund retrieval
-----------------------------------------------------------------------------------

Only request funds for display in the batch update fund selector which the user has permission to use for a purchase order. By limiting the funds to the owner (org unit), we reduce the number of permission checks for unrelated funds that have to be performed on the server (by PCRUD).

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1254918-batch-filter-funds-by-perm

====

FWIW, I have tested Mike's IDL/PCRUD code and all seems to work as expected. Other testers appreciated.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 2.next
Revision history for this message
Bill Erickson (berick) wrote :

I've pushed a combined branch with my sign-off to Mike's PCRUD changes plus my fund filtering UI changes to:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1254918-combined-pcrud-perms-and-fund-filter

All code lives here ^-- now.

Additional testing on the PCRUD changes appreciated, though, since PCRUD is such an important and heavily used component.

Revision history for this message
Jennifer Pringle (jpringle-u) wrote :

We are using this fix live on 2.6 now and our libraries are seeing noticeable speed improvements.

Thanks!!!

Revision history for this message
Ben Shum (bshum) wrote :

Good word from Sitka folks on this, pushing to master for 2.7. Thanks everyone for working on this and testing.

Always nice to see performance gains!

Changed in evergreen:
milestone: 2.next → 2.7.0-alpha
status: Confirmed → Fix Committed
Revision history for this message
Galen Charlton (gmc) wrote :

Bill had also ported this to 2.6: user/berick/lp1254918-combined-pcrud-perms-and-fund-filter-2.6

I've received feedback from one of our customers that the change is also working well for them. Consequently, I've targeted this for pushing to rel_2_6.

Revision history for this message
Galen Charlton (gmc) wrote :

Committed to rel_2_6.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.