SRU/Z39.50 slowness when retrieving holdings

Bug #1609556 reported by Jeff Davis
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Undecided
Unassigned
2.10
Fix Released
Undecided
Unassigned
2.9
Fix Released
Undecided
Unassigned

Bug Description

Evergreen 2.10 (but all supported versions are likely affected)
OpenSRF 2.4
Postgres 9.4

When retrieving records with holdings via SRU, gathering the holdings can be quite slow on a large system. Since the Z39.50 service uses SRU under the hood, that means Evergreen can be quite slow to respond to Z39.50 queries as well.

An SRU search like the following (author search for "dorsey" at a specific org unit, with holdings, limited to 10 results) takes over 15 seconds in our production environment:

/opac/extras/sru/BR1/holdings?version=1.1&operation=searchRetrieve&query=eg.author%3Ddorsey&startRecord=1&maximumRecords=10

A similar search in the OPAC returns results in at most 2-3 seconds. The culprit here seems to be the call to open-ils.supercat.record.basic_holdings.retrieve in OpenILS::WWW::SuperCat->sru_search, which can take more than 1 second per record if you have enough records/holdings in your system.

I have a proposed fix for this issue which I'll share momentarily.

Revision history for this message
Jeff Davis (jdavis-sitka) wrote :

Fix pushed to working branch user/jeffdavis/lp1609556-z3950-sru-slowness:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=0743bcf

This replaces the call to open-ils.supercat.record.basic_holdings.retrieve with a cstore request to unapi.bre. If holdings are requested, we tell unapi.bre to include holdings_xml, and convert the <holdings> element in the response into a set of MARC 852 tags.

Testing this approach on a large system, I see response times in the 2-3 second range (comparable to the OPAC), as opposed to 15+ seconds previously.

tags: added: pullrequest sru supercat z3950
Changed in evergreen:
milestone: none → 2.next
Revision history for this message
Mike Rylander (mrylander) wrote :

Picked to master for 2.11, but leaving for other RMs to decide on the backport. Seems safe enough to me, though.

Changed in evergreen:
status: New → Fix Committed
milestone: 2.next → 2.11-beta
Revision history for this message
Jason Boyer (jboyer) wrote :

Speaking as someone running this patch on 2.9, I'd say it seems pretty safe too. :)

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

While evaluating this for backporting to 2.10, I found that the patch had a (unintended?) side-effect: the 852$d now outputs the circ library's full name, not its description. The patch at the tip of the user/gmcharlt/lp1609556_code_for_circ_lib branch restores the previous behavior. I request that it be applied, as clients may rely on particular values in the 852$d.

Revision history for this message
Mike Rylander (mrylander) wrote :

Thanks, Galen, great catch! Applied to master.

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

I've now backported Jeff's patch and my follow-up to rel_2_10.

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

Should we backport this to rel_2_9? I'm thinking yes, but would like some consensus before I do.

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

+1 for backporting to 2.9

Revision history for this message
Mike Rylander (mrylander) wrote :

+1 from me as well.

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

Backported it to rel_2_9.

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.