Browse search functions need to be 'stable'

Bug #1244432 reported by Dan Wells
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
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;

Tags: pullrequest
Revision history for this message
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."""

Revision history for this message
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 :)

Revision history for this message
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
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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)
tags: added: pullrequest
Revision history for this message
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
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Committed to master! A big thanks to everyone involved!

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