fetching OU settings in batch can be made faster

Bug #1501471 reported by Galen Charlton
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Wishlist
Unassigned

Bug Description

The Dojo patron editor on startup makes a call to open-ils.actor.ou_setting.ancestor_default.batch to retrieve the values of almost 70 library settings. On some systems, this call by itself has been observed to take between 1 and 1.5 seconds to return a response, contributing to the patron editor taking a perceptibly long time to load.

The patch for bug 1424755 updated open-ils.actor.ou_setting.ancestor_default.batch to force a view permissions check for each library setting. This change was needed for security, but since the patron editor does not (at present) require any library settings that have a view permission set, the additional checks are time-consuming.

I propose to tweak open-ils.actor.ou_setting.ancestor_default.batch so that library setting view permissions are checked only when necessary.

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

The working repo branch user/gmcharlt/lp1501471_batch_retrieve_ous_faster contains a two-patch series that

- ensures that OUS visibility is checked only when the setting has a view_perm set
- adds a database function for fetching a bunch of OU settings in one fell swoop; reducing the overhead of multiple cstore requests turns out to be significantly faster

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

Tested and confirmed noticeable speed increase in patron edit load. Pushed a signoff branch:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/berick/lp1501471-aous-batch-signoff

Adding needstest tag for pgtap test.

Revision history for this message
Jason Stephenson (jstephenson) wrote :

Looks like needstest tag did not get added.

I've loaded the sign off branch and while I did not do any speed tests nothing appears to be broken as a result of using it.

I'd push it except for the lack of the test and no pullrequest tag.

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

I've pushed a branch (rebased against master) with Bill's signoff and some pgTAP tests to user/gmcharlt/lp1501471-aous-batch-signoff-with-test in the working/Evergreen repo:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/gmcharlt/lp1501471-aous-batch-signoff-with-test

tags: added: pullrequest
removed: needstest
Revision history for this message
Bill Erickson (berick) wrote :

Testing one more time before merge..

Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
status: New → In Progress
Revision history for this message
Bill Erickson (berick) wrote :

Pgtap and other tests successful. Merged to master. Thanks, Galen!

Changed in evergreen:
status: In Progress → Fix Committed
milestone: none → 2.9.1
milestone: 2.9.1 → 2.next
assignee: Bill Erickson (berick) → nobody
Changed in evergreen:
milestone: 2.next → 2.10-beta
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.