Slowness/timeout on loading bookbags in OPAC

[Jeff Davis]

After upgrading to Evergreen 2.8 and Postgres 9.4, we were seeing very slow load times (20-30 seconds or longer, sometimes timing out with a 504 error) when viewing bookbags in the OPAC.

Eventually we tracked the issue down to a bad query plan when retrieving holdings information:
query: http://pastebin.com/45UCS4yR
explain output: http://explain.depesz.com/s/23Y

I think not everyone will see this problem. For one thing, I'm not sure the JSON output is consistently ordered when it's being converted to SQL, so the query construction may vary. You also need to have a lot of holdings/circs for the query to be slow. But obviously it's an issue in at least some cases.

By modifying the query slightly (specifically, putting the INNER JOIN between acp and acn in parentheses), we can vastly reduce the number of rows at the core of the query. In our environment, that change drops the SQL execution time from 20-30 seconds to 0.1 seconds:
query: http://pastebin.com/spRUGhwR
explain output: http://explain.depesz.com/s/hhU

So, we want to force this query to be constructed in a more sensible way. I'll share a fix momentarily.

[Jeff Davis (jdavis-sitka)]

Proposed fix, closely based on a suggestion from Mike Rylander, is in branch user/jeffdavis/lp1499086-bookbag-slowness in the working repo:

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

Not sure if/how to write a test for this.

[Jeff Davis (jdavis-sitka)]

Pushed a bugfix to the working branch. (Without the fix, holdings display will not be limited by search scope.)

[Kathy Lussier (klussier)]

Jeff asked a question earlier regarding comment #1 about if/how to write a test for this bug fix. I'm not sure if I need to add the needstest tag to this.

Does this fix require a test? If so, can somebody give him some guidance on where to start?

[Dan Wells (dbw2)]

I'm assigning myself to try and review this soon.

[Galen Charlton (gmc)]

I've pushed a squashed, rebased-against-master version of the patch to collab/gmcharlt/lp1499086. Superficially, it appears to work, but I'm not sure that the issue regarding how it affects retrieving bookbags applies anywhere outside of the custom branch that Sitka is running, as basic_opac_copy_query() in stock Evergreen does *not* appear to be invoked anywhere when displaying bookbags. Marking incomplete for now; please feel free to change back if there's context I'm missing.

[Jason Stephenson (jstephenson)]

Removing the targets to 2.8 and 2.9 series based on Galen's comment #5 and a private email exchange with him this morning.

[Jeff Davis (jdavis-sitka)]

Galen is correct that bookbags should not normally be affected by this. It looks like Sitka has some unrelated modifications (for bug 1429317) that result in basic_opac_copy_query being used for bookbags, but that doesn't happen in stock EG.

I think the change to the core query is a useful modification in places where it *is* used, but that discussion properly belongs in bug 1527731.