Bug 36261 - Calculating OpacHiddenItems is too slow in OPAC search
Summary: Calculating OpacHiddenItems is too slow in OPAC search
Status: NEW
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-03-07 00:01 UTC by David Cook
Modified: 2024-04-22 06:57 UTC (History)
2 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2024-03-07 00:01:48 UTC
Calculating items and records hidden by OpacHiddenItems it slow. Too slow.

Fortunately, I've come up with an optimization which seems to effectively eliminate the overhead added by OpacHiddenItems.

Basically, instead of running 1 query per item, I run 1 query per bib to get all hidden items. It's pretty straightforward, but I'd like to battle-test it a bit more. I'm also writing this optimization for an older version of Koha, and I still have more optimizing I want to do, so I'm not going to post my patch just yet...

It's coming though!
Comment 1 Andreas Jonsson 2024-04-17 08:31:21 UTC
Ideally we should resolve this at indexing time so we get the information directly from the search index.

Also the systempreference OpacHiddenItemsHidesRecord could be implemented properly, as it now breaks the result count, and theoretically you could get a blank search result page with a positive result count, and visible results at page two.
Comment 2 David Cook 2024-04-22 00:29:15 UTC
(In reply to Andreas Jonsson from comment #1)
> Ideally we should resolve this at indexing time so we get the information
> directly from the search index.

You can't do it at indexing time, since OpacHiddenItems can be changed at any time. (The OpacHiddenItems feature in general is a bad idea imho.)

> Also the systempreference OpacHiddenItemsHidesRecord could be implemented
> properly, as it now breaks the result count, and theoretically you could get
> a blank search result page with a positive result count, and visible results
> at page two.

I don't think this is possible since the calculation is too complex. Personally, I'd rather deprecate OpacHiddenItemsHidesRecord and just require OpacSuppression to be used for any record that must be completely hidden.
Comment 3 Katrin Fischer 2024-04-22 06:16:14 UTC
We use this feature a lot of the libraries and it always depends on the use case. If you just want to hide something like on order items it works certainly well enough. But maybe it should come with a warning. I'd be against deprecating it.
Comment 4 Andreas Jonsson 2024-04-22 06:37:12 UTC
This is a setting that is rarely changed, I think it is justified to require a reindexing for changes to take effect.
Comment 5 Hugo Agud 2024-04-22 06:40:03 UTC
for us it is also widely used... I should not remove this feature.... on the other hand improve speed or reindex is not a big issue for us.. I do not know big catalogues it takes a lot.
Comment 6 David Cook 2024-04-22 06:51:50 UTC
(In reply to Andreas Jonsson from comment #4)
> This is a setting that is rarely changed, I think it is justified to require
> a reindexing for changes to take effect.

That's a good point. I suppose we do have a number of other settings that require reindexes/restarts. We could add that to the explanation.

That said, I'm still not sure it's possible at indexing time, especially since the search is used by both the OPAC and the staff interface. You couldn't just suppress entire records or items at indexing time without causing systemic problems.
Comment 7 David Cook 2024-04-22 06:55:44 UTC
(In reply to Katrin Fischer from comment #3)
> We use this feature a lot of the libraries and it always depends on the use
> case. If you just want to hide something like on order items it works
> certainly well enough. But maybe it should come with a warning. I'd be
> against deprecating it.

(In reply to Hugo Agud from comment #5)
> for us it is also widely used... I should not remove this feature.... on the
> other hand improve speed or reindex is not a big issue for us.. I do not
> know big catalogues it takes a lot.

It's one of those features that "kinda sorta works - good enough". Widely used but also widely problematic...

By the way, I wasn't talking about deprecating OpacHiddenItems but rather OpacHiddenItemsHidesRecord.

Although in theory if you did calculate OpacHiddenItems at indexing time you could turn on OpacSuppression if OpacHiddenItemsHidesRecord is enabled. That would fix some sins...
Comment 8 David Cook 2024-04-22 06:57:48 UTC
For what it's worth, I've got my performance optimization running in 23.11 production.

It doesn't solve any of the problems of OpacHiddenItemsHidesRecord or facets for hidden items appearing, but it makes a bad situation less bad in terms of performance overheads...