| Summary: | OPAC Search refining by facets is wrong | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Frédéric Demians <f.demians> |
| Component: | OPAC | Assignee: | Henri-Damien LAURENT <henridamien> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | PATCH-Sent (DO NOT USE) | CC: | chris, cmurdock, dpavlin, fridolin.somers, julian.maurice, jwagner, m.de.rooy, nengard, smoreland |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=2614 | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Attachments: |
Proposed patch (based on render method)
Patch Alternative patch with new preference Patch for functionality Install patch Install patch (revised) Patch for functionality (updated) Install patch (updated) |
||
|
Description
Chris Cormack
2010-05-21 01:07:06 UTC
(In reply to comment #0) Hie, I'm currently working with 3.2 alpha 2 version. I also think it is a problem. It can be somehow solved by editing C4/Search.pm line 418 : change : for ( my $j = $offset ; $j < $times ; $j++ ) { to : for ( my $j = 0 ; $j < $size ; $j++ ) { A problem remains : ordering facets with decreasing occurrences order. > It can be somehow solved by editing C4/Search.pm line 418 :
> change : for ( my $j = $offset ; $j < $times ; $j++ ) {
> to : for ( my $j = 0 ; $j < $size ; $j++ ) {
Just somehow. With your solution performance will collapse for large result set. Facet generation must be delegated to the indexing engine which has all the information about terms facets based on resultset and indexes. There is no other efficient solution.
A la revoyure Fridolyn !
Hie, I have worked on a half-solution to increase facets performance in order to use more than 20 results in facets. I first used 500 results for facets : runtime takes 10 seconds. Looking into Search.pm, I found that creating the MARC::Record object was taking most of the time : $facet_record = MARC::Record->new_from_usmarc($record) This method creates a MARC::Record from a ZOOM::Record (returned from Zebra). It populates all fields and subfields. But we only need a few of them for facets. I had to avoid this method. I noticed that a ZOOM::Record can be returned in 2 forms : - record.raw() that is a ISO2709 - record.render() that is a more user-friendly I used then the render form to directly extract facet datas (thanks to Perl regular expressions). Its a real gain of performance : runtime falls at 0,5 second. Despite this performance, I left the limit to the 500 first records. Maybe it should be a system pref. Second change : The number next facet data (facet_count) is the number of occurrences of this value. I changed to this behavior : this number is the number of records that contains the facet. So that when choosing this facet, this number matches the new number of search results. Hope it can help. I'm open to any comment or modification. ZOOM::Record doc: http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#ZOOM::Record Created attachment 2696 [details] [review] Proposed patch (based on render method) http://lists.koha-community.org/pipermail/koha-patches/2010-December/013142.html => I can confirm that Fridolyn trick is efficient and quick but: - Facets count are still wrong. The count are better (ie bigger) but still wrong as soon as result set is larger than examined set used to build facets. - Libraries facet remains misleading. If a biblio record contains a 'foo' library but is at the end of the resultset, 'foo' Library isn't displayed. A patron can conclude that the ressource he's not available at his library. - It reports a huge charge load on Zebra server which has to return a LOT of records to build facets. It should be tested with simultaneous requests in order to see if it impacts Zebra overall performances. (In reply to comment #5) > http://lists.koha-community.org/pipermail/koha-patches/2010-December/013142.html > > => > > I can confirm that Fridolyn trick is efficient and quick but: > > - Facets count are still wrong. The count are better (ie bigger) but still > wrong as soon as result set is larger than examined set used to build > facets. > - Libraries facet remains misleading. If a biblio record contains a 'foo' > library but is at the end of the resultset, 'foo' Library isn't displayed. > A patron can conclude that the ressource he's not available at his library. > - It reports a huge charge load on Zebra server which has to return a LOT of > records to build facets. It should be tested with simultaneous requests in > order to see if it impacts Zebra overall performances. Thanks for feedback. Indeed, increasing the number of records Zebra must return adds an overload to server. Can we imagine in a first time to include the patch but without increasing the number of records : not 500 but the number of records in a page ? I will increase the performance of existing code. Regards, New patches sent by Frederic, awaiting signoff Someone else, familiar with C4::Search, should take a look at Fridolyn Patch. IMHO this patch isn't a complete solution but is already an improvement and doesn't break anything. This bug is mentioned in: Bug 3154 : Facets performance http://lists.koha-community.org/pipermail/koha-patches/2010-November/013100.html Bug 3154: Improving facets perfomance http://lists.koha-community.org/pipermail/koha-patches/2010-November/013101.html This bug is mentioned in: Bug 3154: Improving facets perfomance http://lists.koha-community.org/pipermail/koha-patches/2010-December/013141.html Created attachment 3036 [details] [review] Patch Changed diff output to git patch format; fixed whitespace errors. Pushed to new/bug_3154 I have some worries about the load this places on zebra, can we get some more testing done on it, before I merge it into master (In reply to comment #12) > I have some worries about the load this places on zebra, can we get some more > testing done on it, before I merge it into master I would prefer to make this functionality parameter-driven, as was suggested earlier, via a preference (number of records for building facets). A default of 20 would mimic current behavior. I would not merge it in its current state because the 500 is hardcoded and could affect performance substantially in larger catalogs with large result sets. (In reply to comment #13) Hie, I totally agree, a system pref is perfect to choose the limit depending on the server's performance. But I never add a system pref, it includes many modifications I've seen (update script, database structure in many languages, ...). Could someone manage this? I'm also quite busy at staff. Regards, I have a question about what the sys pref would control. Will is control the showing of the numbers next to the facets? Or will it control the showing of all possible facets on the first page of results? If both then I'd ask for two prefs instead of one to control both. I don't mind giving up the numbers (personally) but I always thought that the facets were showing me facets for all of the results (and was kind of disappointed to find out that the facets are in fact inaccurate). (In reply to comment #15) > I have a question about what the sys pref would control. Will is control the > showing of the numbers next to the facets? Or will it control the showing of > all possible facets on the first page of results? If both then I'd ask for two > prefs instead of one to control both. I don't mind giving up the numbers > (personally) but I always thought that the facets were showing me facets for > all of the results (and was kind of disappointed to find out that the facets > are in fact inaccurate). I propose: FacetFirstBiblios: to fix the number of biblio records used to find facets and count. FacetHideCount: to hide numbers next to the facets. And someone who isn't busy has to do it :-) (In reply to comment #16) > FacetHideCount: to hide numbers next to the facets. Do not completely understand that one. I checked the git log on opac facets template. And discovered that at creation time of opac-facets.inc, the facet count was already commented and thus invisible. See commit: f8e9fb6445dadbdef91e13a253c9c6bcb75e0eec I can understand why the number was made invisible. As long as it does not refer to the complete result set, it is quite irrelevant. Based on the work of Fridolyn and Frederic latest patch with a default of 50, I have attached a patch with a new preference maxRecordsForFacets. My default would be 20 to mimic current behavior. Created attachment 3201 [details] [review] Alternative patch with new preference Introduces pref maxRecordsForFacets (In reply to comment #17) > (In reply to comment #16) > > FacetHideCount: to hide numbers next to the facets. > > Do not completely understand that one. I checked the git log on opac > facets template. And discovered that at creation time of > opac-facets.inc, the facet > count was already commented and thus invisible. > See commit: f8e9fb6445dadbdef91e13a253c9c6bcb75e0eec > > I can understand why the number was made invisible. As long as it does > not refer to the complete result set, it is quite irrelevant. That's why a syspref may be useful in order to let the library choose whether or not a count is displayed next to each facet. If you choose via the first syspref to find your facets on the 1000 first records of a resultset, it may be considered as relevant to display a count. Created attachment 3222 [details] [review] Patch for functionality Contains functionality with two prefs. Created attachment 3223 [details] [review] Install patch Install stuff only: sql files and updatedatabase script. (In reply to comment #19) > That's why a syspref may be useful in order to let the library choose > whether or not a count is displayed next to each facet. If you choose via > the first syspref to find your facets on the 1000 first records of a > resultset, it may be considered as relevant to display a count. OK. I am convinced. Sending two patches this time. The first patch contains functionality including second pref displayFacetCount. The second patch only contains install stuff. Could one of you please test and sign? Created attachment 3224 [details] [review] Install patch (revised) Installation issues (small revision) (In reply to comment #23) > Created attachment 3224 [details] [review] > Install patch (revised) > > Installation issues (small revision) Tanks a lot for your help. Regards, This bug is mentioned in: Bug 3154: Improving facets perfomance http://lists.koha-community.org/pipermail/koha-patches/2011-February/013878.html Bug 3154 Re-activate facet count http://lists.koha-community.org/pipermail/koha-patches/2011-February/013879.html Bug 3154 Limit facets computing on first 50 biblios http://lists.koha-community.org/pipermail/koha-patches/2011-February/013910.html Created attachment 3332 [details] [review] Patch for functionality (updated) Created attachment 3334 [details] [review] Install patch (updated) Pushed, please test This bug is mentioned in: Bug 3154: Preferences for building facets http://lists.koha-community.org/pipermail/koha-patches/2011-March/014267.html Bug 3154: Preferences for building facets http://lists.koha-community.org/pipermail/koha-patches/2011-March/014268.html |