Browse search functions need to be 'stable'

Bug #1244432 reported by Dan Wells on 2013-10-24
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Critical
Unassigned

Bug Description

Finally got a chance to test 2.5 browse on our full production database. Unfortunately, performance was unusably poor (I stopped the browse query after 32 minutes).

After some troubleshooting, it seems we need a couple helper functions to be set as STABLE (original suggestion was IMMUTABLE, but that was pointed out to be incorrect by Dan S.):

metabib.browse_bib_pivot()
metabib.browse_authority_refs_pivot()

After doing this, I got what appeared to be correct results in a couple seconds. This change makes sense to me, but since this is nothing I am expert in, I wanted to post this for general feedback before offering a branch of any kind. You can update these functions in an existing DB like so:

ALTER FUNCTION metabib.browse_bib_pivot (integer[], text) STABLE;
ALTER FUNCTION metabib.browse_authority_refs_pivot (integer[], text) STABLE;

Dan Scott (denials) wrote :

Unfortunately I don't think these functions are, strictly speaking, mutable.

The first argument to metabib.browse_bib_pivot, integer[], refers to the metabib.browse_entry_def_map IDs. Given that the corresponding entry in metabib.browse_entry_def_map could change between calls, it is not mutable. Those entries may not be likely to change - but if they do, it would require a restart of PostgreSQL to get the change recognized if the functions are marked IMMUTABLE.

I believe metabib.browse_authority_refs_pivot suffers from a similar concern.

Per the PostgreSQL docs:

"""IMMUTABLE indicates that the function cannot modify the database and always returns the same result when given the same argument values; that is, it does not do database lookups or otherwise use information not directly present in its argument list."""

Dan Scott (denials) wrote :

s/it is not mutable/it is mutable/ -- way to boldly assert the exact opposite of what I was trying to say - heh :)

Dan Wells (dbw2) wrote :

Thanks, Dan. Sorry, I was thinking IMMUTABLE functions were only not allowed to write to the DB, but that was an incorrect recollection. In this case, STABLE seems sufficient to solve the problem, and I don't think it suffers from the same issues. I will update the original description.

summary: - Browse search functions need to be 'immutable'
+ Browse search functions need to be 'stable'
description: updated
Jason Stephenson (jstephenson) wrote :

So, like, does someone want to add a branch? I'd be more than happy to test it since I seem to have browse queries running for days.

Dan Wells (dbw2) wrote :

Jason, thanks for the interest in testing. I was hoping for some more thumbs-up or thumbs-down type feedback before making a branch, but I also want to move this along any way I can, so I'll post a branch ASAP. You could also get a head start on testing if you wish, as the entirety of the branch (as far as upgrades go) will be the two ALTER FUNCTION statements in the original bug report.

Dan Wells (dbw2) wrote :

Ok, branch up at:

http://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dbwells/lp1244432_browse_functions_stable

working/user/dbwells/lp1244432_browse_functions_stable

I decided to be proactive and made the other two browse pivot functions also STABLE. One I am not sure is even in use, and the other is just a combo of the first two, so it makes sense.

Dan Wells (dbw2) on 2013-10-29
tags: added: pullrequest
Chris Sharp (chrissharp123) wrote :

I can confirm that these changes make catalog browsing usable when it wasn't before. Thanks, Dan!

Sign-off branch at user/csharp/lp1244432_browse_functions_stable

Changed in evergreen:
status: New → Confirmed
Jason Stephenson (jstephenson) wrote :

Committed to master! A big thanks to everyone involved!

Changed in evergreen:
status: Confirmed → Fix Committed
Dan Wells (dbw2) on 2013-11-11
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