fetching OU settings in batch can be made faster

Bug #1501471 reported by Galen Charlton on 2015-09-30
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
Evergreen
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.

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

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.

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
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
Bill Erickson (berick) wrote :

Testing one more time before merge..

Changed in evergreen:
assignee: nobody → Bill Erickson (berick)
status: New → In Progress
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  Edit
Everyone can see this information.

Other bug subscribers