Search indexes should use GIN by default

Bug #1703658 reported by Kathy Lussier
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Low
Unassigned
3.5
Fix Released
Low
Unassigned
3.6
Fix Released
Low
Unassigned

Bug Description

We often recommend using GIN metabib indexes to Evergreen sites as a way to improve performance of Evergreen searches. However, the default in Evergreen is to use GIST. If GIN is recommended for better performance, we should make it the default.

Given that a reingest is required to make this change, could we make this change the next time we have a feature that requires a full reingest anyway?

Jason Boyer (jboyer)
Changed in evergreen:
status: New → Confirmed
Revision history for this message
Jason Boyer (jboyer) wrote :

At long last a branch appears from the mists: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/jboyer/lp1703658_gindexes / working/user/jboyer/lp1703658_gindexes

Go forth and get fast!

To Kathy's note above I don't believe a reingest is required; this is simply rebuilding the indexes for the tables that hold the ingested data.

tags: added: pullrequest
Changed in evergreen:
milestone: none → 3.7-beta
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I just started working on converting from GIST to GIN on a test database server.

I'm unclear on Kathy's statement that this requires a reingest. I don't see how it does, since the upgrade code is to drop the GIST index and create the GIN index. If someone knows why this would require a reingest, please comment on this bug.

NB: By "reingest," I assume anything that would require running the database functions used by the pingest.pl support script.

This also appears to be something that we could not do in our typical DB upgrade scripts. It would be good to include a small Perl utility to either write out the code to drop and create the indexes or to do the "conversion" itself. In my test case, I dumped the schemaname and indexdef from pg_indexes for the GIST indexes, and then used a regexp search and replace to get the SQL needed to drop the old and create the new indexes.

Jason Boyer (jboyer)
no longer affects: evergreen/3.7
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I fixed an error in the upgrade script, added release notes, and pushed my signoff branch to here:

https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/dyrcona/lp1703658_gindexes-signoff

Thanks, Jason, for the upgrade script!

tags: added: signedoff
Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
Revision history for this message
Jason Stephenson (jstephenson) wrote :

I have a couple of quick things to add:

1. We applied this to our training server late last week after doing some timed searches. The same searches were tested again after this branch was applied to the database, and we saw a significant improvement in search response times as well as more consistent response times.

2. Should this bug still be targeted at 3.5 and 3.6? The importance of Wishlist indicates this is a a new feature, and it seems like a new feature to me.

I would just go ahead and push this, but I think we need to decide if this is a new feature or a performance bug fix, first.

Revision history for this message
Rogan Hamby (rogan-hamby) wrote :

my .02 dinars

We could have a long debate about older code not taking advantage of newer features and whether it is a bug or not and while that might be fun in the right time and place I think this has enough benefit to a core function that it's worth treating as a bug to backport regardless of what outcome we would have in a lexical semantic debate.

Changed in evergreen:
importance: Wishlist → Low
Changed in evergreen:
assignee: nobody → Jason Stephenson (jstephenson)
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Rogan's point taken.

I have pushed this to master, rel_3_6, and rel_3_5 for inclusion in future upgrades.

Thanks Kathy, Jason, and Rogan!

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
status: Confirmed → Fix Committed
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.