When we switched from ISO2709 to MARCXML as the default indexing format we got rid of the errors regarding big record indexing; notably serials with lots of items. The problem is that the current ZOOM connection in Koha is set to retreive records in 'usmarc'. This makes retreiving search results silently fail for big records, and even when those records are correctly indexed, the user would fail to reach them. Steps to reproduce: - Create a new simple record with 100$a = Arquitectura viva - Add it several big note fields - Add several items to it - Reindex, search for Arquitectura viva - Record doesn't show - In a terminal, use yaz-client to do a manual query (adjust to your setup): $ yaz-client -k 100000 unix:/home/tomas/koha-dev/var/run/zebradb/bibliosocket > base biblios > f @attr 1=4 @attr 4=1 @attr 6=3 "Arquitectura viva" > s It should fail because of the size of the record. - Rerun like this: > format xml > elem marcxml > f @attr 1=4 @attr 4=1 @attr 6=3 "Arquitectura viva" > s There you go :-D
Created attachment 22291 [details] [review] Bug 11096: Remove use of MARC::File::USMARC from AuthoritiesMarc.pm Thats it. To test: - Apply the patch - Test authorities search in all its flavours (staff and opac). - Use the dontmerge syspref to try the AuthoritiesMarc::merge function. Regards To+ Sponsored-by: Universidad Nacional de Cordoba
Created attachment 22292 [details] [review] Bug 11096: force XML syntax for Zconn This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba
I've applied the patches, but I'm still unable to retrieve the big record from Zebra. I created a new bib record with an excessive number of 5xx fields and added many items, and it has created problems with the MARC record leader. This is the leader for a small record: <leader>03322cam a2200469 a 4500</leader> Here is the (broken) leader for a big record (without items) in MySQL: <leader>143530nama22006133 45000</leader> Here is the (broken) leader for a big record (with items) in Zebra (accessed via YAZ): <leader>584260nama22019693 4500</leader> As you can see, both of the broken leaders are quite broken. The MySQL version is 25 characters rather than 24. In Zebra, it has 24 positions, but much of the data are in the wrong place because of that 6 character record length. -- When I try to access the record in Yaz-client, I receive the following error: <!-- Directory offset 276: Data out of bounds 59725 >= 58426 --> </record> As Tomas pointed out, 584260 has been shortened to 58426 as Yaz is expecting 5 characters rather than 6 characters for the record length. -- So...at the moment...it seems that indexing and retrieving oversized records doesn't work. If you try to bring up a detail page manually, you can, but the XSLTs won't work correctly as it can't rely on the leader positions correctly. A "book" might show up as a "set". In one case, some of the XSLT display isn't even showing because the data that is mixed around is totally unusable. -- Admittedly, the most likely causes of big records are notes and items, so perhaps it would make sense to separate out the item records from the bib records when indexing. As for the notes...I'm not sure how we would handle that. While there is a 9999 character limit on a note field, you could have a hundred note fields that break your record. -- I'm not really sure what the answer is, but here is the problem.
Martin reminded me that we thought about changing the leader on the fly...we could check for the length of the record and if it is too long, then we could replace it with xxxxxx or spaces or low numbers or something like that. We would need to take this into account anytime we were ingesting or disseminating MARC records though...
By the coordinated efforts of Galen, Jared, Thomas, Tomas, and myself, we've found that the problems that I was experiencing were due to using version 2.0.3-1 of MARC::Record and the GRS-1 indexing filter. Newer versions of MARC::Record will set the record length of the leader to 99999 if it exceeds 99999. Here is the commit that changed MARC::Record: https://sourceforge.net/p/marcpm/code/ci/3efc45777f8e4bec1cd09beb6ab2968e5fb0e44f/ So, upgrading to a newer version of MARC::Record will fix this problem when using DOM. However, these problems will persist when using GRS-1 (as it seems that GRS-1 relies on the ISO2709 spec in some way internally). As Jared mentioned on IRC, the best bet is probably to push this patch, even though it will only work for DOM indexing. GRS-1 indexing will just work the same as always, so there is no negative impact for GRS-1 users for using this patch.
David, can you sign off on this?
Created attachment 22821 [details] [review] Bug 11096: Take advantage of MARC::Field->as_string feature Avoid looping over the subfields for building the facets. Sponsored-by: Universidad Nacional de Cordoba
Created attachment 22845 [details] [review] Bug 11096: Take advantage of MARC::Field->as_string feature Avoid looping over the subfields for building the facets. pushes higher version dependencies for MARC::File::XML and MARC::Record. Apply on top of the other patches for this bug, facets should still show as usual. Sponsored-by: Universidad Nacional de Cordoba
Created attachment 22846 [details] Sample record (lots of items) This is a sample record with lots of items and note fields. To use download and do: - Import the record: $ misc/migration_tools/bulkmarcimport.pl -v -b -m=marcxml -file=sample_record.marcxml $ misc/migration_tools/rebuild_zebra.pl -b -a -z -v -x - Have zebrasrv running, for example like this: $ zebrasrv -f ~/koha-dev/etc/koha-conf.xml -k 10240000 -v "fatal,warn,log" - Do a search for "Arquitectura viva" You get no results - Apply the patch series - Re-do the search Voila
Tomás, the second patch conflicts here <<<<<<< HEAD my $marcrecord = eval { MARC::File::USMARC::decode( $marcresults->[$i] ); }; if ( $@ ) { warn "ERROR DECODING RECORD - $@: " . $marcresults->[$i]; next; } ======= my $marcrecord = MARC::Record->new_from_xml( $marcresults->[$i], 'UTF-8' ); >>>>>>> Bug 11096: force XML syntax for Zconn I think because the eval was recently introduced, should we be evaling for new_from_xml or does it handle errors better than MARC::File::USMARC::decode does?
Created attachment 22997 [details] [review] Bug 11096: force XML syntax for Zconn This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba
(In reply to Chris Cormack from comment #10) > Tomás, the second patch conflicts here > > <<<<<<< HEAD > my $marcrecord = eval { MARC::File::USMARC::decode( > $marcresults->[$i] ); }; > if ( $@ ) { > warn "ERROR DECODING RECORD - $@: " . $marcresults->[$i]; > next; > } > > ======= > my $marcrecord = MARC::Record->new_from_xml( > $marcresults->[$i], > 'UTF-8' > ); > >>>>>>> Bug 11096: force XML syntax for Zconn > > I think because the eval was recently introduced, should we be evaling for > new_from_xml or does it handle errors better than MARC::File::USMARC::decode > does? MARC::File::XML->new_from_xml implicitly calls its own decode method, which also croaks if problems arise decoding. So I attached a rebased patch that preserves that introduced behaviour (I mean the eval).
Created attachment 23030 [details] [review] Squashed patch. Regards To+ Sponsored-by: Universidad Nacional de Cordoba
Created attachment 23033 [details] [review] Bug 11096: wrap MARC::Record->new_from_xml call with eval Catch situations where MARC::Record would croak for bad-formed records. To test: - Run prove t/db_dependent/Search.t - It will fail at test 65 [1] - Apply the patch, it will warn for some failing records, but the tests will run smoothly. Regards To+ Sponsored-by: Universidad Nacional de Cordoba [1] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684)
Created attachment 23234 [details] [review] [PASSED QA] Bug 11096: force XML syntax for Zconn and remove MARC::File::USMARC everywhere. This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. References to MARC::File::USMARC where replaced for the corresponding MARC::File::XML counterpart from AuthoritiesMarc.pm. - Authorities search should work as usual. - Set dontmerge to see AuthoritiesMarc::merge still works. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on second patch.
Created attachment 23235 [details] [review] [PASSED QA] Bug 11096: wrap MARC::Record->new_from_xml call with eval Catch situations where MARC::Record would croak for bad-formed records. To test: - Run prove t/db_dependent/Search.t - It will fail at test 65 [1] - Apply the patch, it will warn for some failing records, but the tests will run smoothly. Regards To+ Sponsored-by: Universidad Nacional de Cordoba [1] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> - All tests and QA script pass after applying the patch. - Verfied that sample record is not searchable before applying patch, but can be searched without problems after applying it. - Tested authority searching still workds as expected. - Tested bilbiographic searching still works as expected. - Simple search, also truncated search terms - Facetting - Advanced search, also itemtype and pubyear limits - Tested dontmerge preference, editing a linked authority and confirming the change is made to the linked records as well.
Created attachment 23236 [details] [review] [SIGNED OFF] Bug 11096: force XML syntax for Zconn and remove MARC::File::USMARC everywhere. This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. References to MARC::File::USMARC where replaced for the corresponding MARC::File::XML counterpart from AuthoritiesMarc.pm. - Authorities search should work as usual. - Set dontmerge to see AuthoritiesMarc::merge still works. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on second patch.
Created attachment 23237 [details] [review] [SIGNED OFF] Bug 11096: wrap MARC::Record->new_from_xml call with eval Catch situations where MARC::Record would croak for bad-formed records. To test: - Run prove t/db_dependent/Search.t - It will fail at test 65 [1] - Apply the patch, it will warn for some failing records, but the tests will run smoothly. Regards To+ Sponsored-by: Universidad Nacional de Cordoba [1] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> - All tests and QA script pass after applying the patch. - Verfied that sample record is not searchable before applying patch, but can be searched without problems after applying it. - Tested authority searching still workds as expected. - Tested bilbiographic searching still works as expected. - Simple search, also truncated search terms - Facetting - Advanced search, also itemtype and pubyear limits - Tested dontmerge preference, editing a linked authority and confirming the change is made to the linked records as well.
Created attachment 23336 [details] [review] Bug 11096: force XML syntax for Zconn and remove MARC::File::USMARC everywhere. This patch makes Koha <-> Zebra exchange speak XML instead of the current 'USMARC'. For that, it changes some bits in - C4::Context->Zconn - C4::Context-_new_Zconn Zebra's 'element' variable is set to "marcxml" to match our zebra config files. References to MARC::File::USMARC::decode are substituted for calls to the correct one [1] (i.e. MARC::Record::new_from_xml using an explicit 'UTF-8' encoding for the task. This patch also removes calls to the unsafe ZOOM::Record->render()[2] method. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). - The FindDuplicates function was MARCXML-ized too. - Unit tests have been moved to expect MARCXML from Zebra as well. TODO: relevance still uses USMARC. Need advise on that. References to MARC::File::USMARC where replaced for the corresponding MARC::File::XML counterpart from AuthoritiesMarc.pm. - Authorities search should work as usual. - Set dontmerge to see AuthoritiesMarc::merge still works. Regards To+ [1] As a side effect some encoding problems got fixed in facets and record details. http://search.cpan.org/~gmcharlt/MARC-XML-1.0.1/lib/MARC/File/XML.pm#decode() [2] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Testing notes on second patch. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Works as advertised. Passes koha-qa.pl, t, and xt.
Created attachment 23337 [details] [review] Bug 11096: wrap MARC::Record->new_from_xml call with eval Catch situations where MARC::Record would croak for bad-formed records. To test: - Run prove t/db_dependent/Search.t - It will fail at test 65 [1] - Apply the patch, it will warn for some failing records, but the tests will run smoothly. Regards To+ Sponsored-by: Universidad Nacional de Cordoba [1] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> - All tests and QA script pass after applying the patch. - Verfied that sample record is not searchable before applying patch, but can be searched without problems after applying it. - Tested authority searching still workds as expected. - Tested bilbiographic searching still works as expected. - Simple search, also truncated search terms - Facetting - Advanced search, also itemtype and pubyear limits - Tested dontmerge preference, editing a linked authority and confirming the change is made to the linked records as well. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This has run into a snag. While reviewing this, I decided to write a direct regression test, which can be found here: http://git.librarypolice.com/?p=koha-galen.git;a=commit;h=cffafe30284f65ef21fc315117b5d7abb39483dc This patch applies on both the current master as well as master with the patches for bug 11096 applied. On current master, the new tests *work* for MARC21 if the indexing mode is GRS-1, but, as expected, fail if the indexing mode is DOM. On top of the patches for this bug, the new tests work if the indexing mode is DOM. Yay! Here's the catch: they fail, and fail with a crash, if the indexing mode is GRS-1. This is because Zebra itself returns a Z39.50 error 14 (failure while presenting record) in line 493 of Search.pm: $record = $results[ $i - 1 ]->record($j)->raw(); Because Zebra refuses to render an invalid ISO2709 blob MARC XML, ->record($j) returns undef, and the crash occurs when it tries to run undef->raw(). Unfortunately, it's not a matter of just wrapping an eval() around it and carrying on. Here's the situation today: Over-large record in Zebra with GRS-1: can retrieve it ... with DOM: can't retrieve it Note that it works for GRS-1 mostly because (a) Zebra doesn't try to do any conversions; it think it's storing iso2709, and if you want iso2709 back, it just sends it and (b) MARC::Record is pretty forgiving. Here's the situation with the current proposed patches: ... with GRS-1: can't retrieve, and it crashes the search ... with DOM: it works To sum up: this patch would cause a regression for GRS-1 users that happen to have over-large records in their database. I see couple ways to solve this. One would be to simply immediately desupport GRS-1 -- but I don't think our UNIAMRC users are quite there yet. The other way would be to make the code adjust how it retrieves records, and whether to use the USMARC or the XML decoder, based on the values of the zebra_bib_index_mode and zebra_auth_index_mode settings.
Created attachment 23366 [details] [review] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com>
(In reply to Galen Charlton from comment #21) > This has run into a snag. While reviewing this, I decided to write a direct > regression test, which can be found here: > > http://git.librarypolice.com/?p=koha-galen.git;a=commit; > h=cffafe30284f65ef21fc315117b5d7abb39483dc > > This patch applies on both the current master as well as master with the > patches for bug 11096 applied. I've also attached the regression test patch to this bug.
Created attachment 24019 [details] [review] [SIGNED-OFF] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Created attachment 24020 [details] [review] Bug 11096 - Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba
Created attachment 24029 [details] [review] [SIGNED OFF] Bug 11096: force XML syntax for Zconn and remove MARC::File::USMARC everywhere.
Created attachment 24030 [details] [review] [SIGNED OFF] Bug 11096: wrap MARC::Record->new_from_xml call with eval
Created attachment 24031 [details] [review] Bug 11096: Make use of MARCXML conditional of DOM being set We have a way of knowing the indexing mode that is in use: C4::Context->config('zebra_bib_index_mode') and C4::Context->config('zebra_auth_index_mode') this patch makes Koha use that to fallback to the old USMARC serialization format for retreiving from Zebra in case we are on a GRS-1 setup. To test. Check tests now pass: $ prove -v t/db_dependent/Search.t Sponsored-by: Universidad Nacional de Cordoba
Made the use of MARCXML for serialization dependent on having DOM set as Galen proposed on comment #21. Tests now pass.
Please recall that I made the bug depend on bug 11499 (fixes correctly setting indexing mode in the test scripts) so apply the patches in order, on top of the patches for that bug.
Created attachment 24172 [details] [review] Bug 11096: force XML syntax for Zebra in DOM I was asked to squash the patches, so here we go. I removed the signoffs they had because I know you will sign them again :-P To+ Sponsored-by: Universidad Nacional de Cordoba
Created attachment 24186 [details] [review] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 24187 [details] [review] Bug 11096 - Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 24188 [details] [review] Bug 11096: force XML syntax for Zebra in DOM This patch makes Koha <-> Zebra use MARCXML for the serialization. We currently use the USMARC, which remains for GRS-1. This functions are modified for that purpose: - C4::Context->Zconn - C4::Context-_new_Zconn - C4::Search->* - C4::AuthoritiesMarc->SearchAuthorities - C4::AuthoritiesMarc->merge - C4::AuthoritiesMarc->FindDuplicateAuthority C4::Context->config('zebra_bib_index_mode') and C4::Context->config('zebra_auth_index_mode') Calls to the unsafe ZOOM::Record->render()[2] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Created attachment 24490 [details] [review] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 24491 [details] [review] Bug 11096 - Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 24492 [details] [review] Bug 11096: force XML syntax for Zebra in DOM This patch makes Koha <-> Zebra use MARCXML for the serialization. We currently use the USMARC, which remains for GRS-1. This functions are modified for that purpose: - C4::Context->Zconn - C4::Context-_new_Zconn - C4::Search->* - C4::AuthoritiesMarc->SearchAuthorities - C4::AuthoritiesMarc->merge - C4::AuthoritiesMarc->FindDuplicateAuthority C4::Context->config('zebra_bib_index_mode') and C4::Context->config('zebra_auth_index_mode') Calls to the unsafe ZOOM::Record->render()[2] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Passes koha-qa.pl, all unit test. Works as advertised.
Works great! On a side note, search results are crazy slow for the example record Tomás uploaded. This is not an issue with this patch but does expose how slowly Koha process all items on a search results. I've seen this before with records that have hundreds of items that don't surpass the marc record length. Records that are overly large due to bib level notes fields and such will not trigger this kind of slowness. It may behoove us to find any efficiencies we can for item processing for search results.
To add another set of eyes to this I have done some tests today and I found a problem in the rewrite of the facetting code. Without the patches the item type and other item related facets show the number of records related to them, but with the patch, the number of occurrences of the tag/items is shown. I think this is an unwanted change in behaviour that can be confusing to patrons. We shouldn't mix number of items and number of records in the facets. I think it's related to this change: unless ( $data ~~ @used_datas ) { - $facets_counter->{ $facet->{idx} } - ->{$data}++; push @used_datas, $data; } - } # fields + + $facets_counter->{ $facet->{idx} }->{$data}++; + + } # fields I will do more tests, but failing QA to get this fixed.
TESTING NOTES MARC21 - b:dom - a:dom 1- Reindexing OK Full bib reindex -b -x -r -v OK Full auth reindex -a -x -r -v OK Update record OK Delete record 2- Catalog search OK Searching OK big record can be found and displayed 3- Facets OK limiting on several facets !!! facet counts 4- Authority search OK Searching OK Update record OK Delete record 5- prove t/db_dependent/Search.t OK All tests pass Problems found: Item related facets show the "wrong" count as what you expect would be a record related count, but what you see is the item related count Example: Itemtype: Books (15) can be only 5 records Typo in documentation serhaps/perhaps introduced by third patch. I am going to try and repeat the tests on a UNIMARC bib:grs1, auth:dom installation next.
UNIMARC - b:grs1 - a:dom The patches break search in my UNIMARC installation. A normal search for the keyword "der" that gives results on my master branch with freshly reindexed records, fails on the branch with patches applied, reindex and restart of the zebra server: Error: Can't call method "raw" on an undefined value at /home/katrin/kohacloneU/C4/Search.pm line 503. Authority search seems to work ok.
Created attachment 24580 [details] [review] Bug 11096: (qa followup) facets count and UNIMARC record syntax - An error in a copy and paste operation made me count facet occurences instead of records where the fields where present. - As of bug 3087 UNIMARC uses the 'unimarc' record syntax. I assumed 'usmarc' was used as serialization format as thought it was dialect-agnostic. This patch adds a test of whether we are in UNIMARC context or not and sets the record syntax to match the one from koha-conf.xml. Regards To+
Sorry Tomás, bad news. I get the 'Can't call method "data" on an undefined value' error with and without the last patch. Marked as Failed QA.
Jonathan, can u tell me how is your setup? Specifically in koha-conf.xml do you have: <retrievalinfo> <retrieval syntax="unimarc" name="F"/> ? or <retrievalinfo> <retrieval syntax="usmarc" name="F"/> I assume you have this in your config section: <zebra_bib_index_mode>grs1</zebra_bib_index_mode> <zebra_auth_index_mode>dom</zebra_auth_index_mode>
(In reply to Tomás Cohen Arazi from comment #44) > Jonathan, can u tell me how is your setup? Specifically in koha-conf.xml do > you have: > > <retrievalinfo> > <retrieval syntax="unimarc" name="F"/> > ? or > <retrievalinfo> > <retrieval syntax="usmarc" name="F"/> > > I assume you have this in your config section: > > <zebra_bib_index_mode>grs1</zebra_bib_index_mode> > <zebra_auth_index_mode>dom</zebra_auth_index_mode> Hum, I did not have the zebra_bib_index_mode defined. In your patch, in C4::Search the test is done on zebra_bib_index_mode eq 'dom' but in C4::Context it is done on 'grs1'. I think it should be done every where on 'dom'. That way, if the value does not exist, we assume it is a dom instance. Tomás, please signoff if you agree.
Created attachment 24602 [details] [review] Bug 11096: (qa followup) facets count and UNIMARC record syntax - An error in a copy and paste operation made me count facet occurences instead of records where the fields where present. - As of bug 3087 UNIMARC uses the 'unimarc' record syntax. I assumed 'usmarc' was used as serialization format as thought it was dialect-agnostic. This patch adds a test of whether we are in UNIMARC context or not and sets the record syntax to match the one from koha-conf.xml. Regards To+ Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 24603 [details] [review] Bug 11096: If zebra_bib_index_mode is not defined, assume it is grs1. Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 24604 [details] [review] Bug 11096: (qa followup) facets count and UNIMARC record syntax - An error in a copy and paste operation made me count facet occurences instead of records where the fields where present. - As of bug 3087 UNIMARC uses the 'unimarc' record syntax. I assumed 'usmarc' was used as serialization format as thought it was dialect-agnostic. This patch adds a test of whether we are in UNIMARC context or not and sets the record syntax to match the one from koha-conf.xml. Regards To+
Created attachment 24605 [details] [review] Bug 11096: fallback to the correct defaults if config entries missing My mistake choosing the right patch...
Created attachment 24607 [details] [review] Bug 11096: fallback to the correct defaults if config entries missing As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like: - bib_index_mode (defaults to 'grs1' if not specified) - auth_index_mode (defaults to 'dom') we should do the same in C4::Context. So people having old config files have a seamless behaviour. Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com> Without this patch, I got Can't call method "data" on an undefined value at /home/koha/src/C4/Search.pm line 1740. without zebra_bib_index_mode entry in my koha config file. With this patch, the search works as before.
Tomas, I am sorry, but I have problem to resolve the conflict in Search.t in the first patch and have the following patches apply. Can you please take a look?
Created attachment 24731 [details] [review] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 24732 [details] [review] Bug 11096 - Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Hm, the tests are failing for me: ... t/db_dependent/Search.t .. 4/4 # Looks like you failed 2 tests of 4. t/db_dependent/Search.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/4 subtests Test Summary Report ------------------- t/db_dependent/Search.t (Wstat: 512 Tests: 4 Failed: 2) Failed tests: 1-2 Non-zero exit status: 2
(In reply to Katrin Fischer from comment #54) > Hm, the tests are failing for me: > ... > t/db_dependent/Search.t .. 4/4 # Looks like you failed 2 tests of 4. > t/db_dependent/Search.t .. Dubious, test returned 2 (wstat 512, 0x200) > Failed 2/4 subtests > > Test Summary Report > ------------------- > t/db_dependent/Search.t (Wstat: 512 Tests: 4 Failed: 2) > Failed tests: 1-2 > Non-zero exit status: 2 Are you sure you're on latest master with all the patches applied? It is passing for me http://snag.gy/fHAsw.jpg
Yes, sure of both. Hm, so we need to find out where the difference is.
(In reply to Katrin Fischer from comment #56) > Yes, sure of both. Hm, so we need to find out where the difference is. MARC::Record version 2.0.6 is needed
Is it packaged yet?
(In reply to Katrin Fischer from comment #58) > Is it packaged yet? In my repo, it is :-D http://ubuntu.unc.edu.ar/koha/
Installed it from CPAN - tests still failing.
not ok 48 - getRecords on _ALLRECORDS PQF returned all records not ok 53 - Weighted query returns best match first # TODO Query weighting does not behave exactly the same in DOM vs. GRS not ok 100 - can do a search that retrieves an over-large bib record (bug 11096) not ok 101 - able to render over-large bib record (bug 11096) not ok 102 - able to render over-large bib record (bug 11096) not ok 103 - able to render over-large bib record (bug 11096)
The first 2 tests also exist on the master branch and pass there.
(In reply to Katrin Fischer from comment #62) > The first 2 tests also exist on the master branch and pass there. Can you email me your dev setup? I cannot reproduce the problems you refer to, and hence cannot provide a workaround. tomas@zoe:koha-community-src(clean_master)$ git fetch tomas@zoe:koha-community-src(clean_master)$ git reset --hard origin/master HEAD is now at 27c312b Bug 11533: fix authority searching with no sorting when QueryParser is enabled tomas@zoe:koha-community-src(clean_master)$ git bz apply 11096 Bug 11096 - Koha cannot retrieve big records from Zebra 24492 - Bug 11096: force XML syntax for Zebra in DOM 24602 - Bug 11096: (qa followup) facets count and UNIMARC record syntax 24607 - Bug 11096: fallback to the correct defaults if config entries missing 24731 - Bug 11096: (follow-up) add direct regression tests 24732 - Bug 11096 - Wrap each test set configuration into a subtest Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 11096: force XML syntax for Zebra in DOM Applying: Bug 11096: (qa followup) facets count and UNIMARC record syntax Applying: Bug 11096: fallback to the correct defaults if config entries missing Applying: Bug 11096: (follow-up) add direct regression tests Applying: Bug 11096 - Wrap each test set configuration into a subtest tomas@zoe:koha-community-src(clean_master)$ prove t/db_dependent/Search.t 2> /dev/null t/db_dependent/Search.t .. ok All tests successful. Files=1, Tests=4, 29 wallclock secs ( 0.09 usr 0.01 sys + 20.84 cusr 2.72 csys = 23.66 CPU) Result: PASS tomas@zoe:koha-community-src(clean_master)$
What exactly do you need?
Created attachment 24874 [details] [review] Bug 11096: (follow-up) add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Created attachment 24894 [details] [review] [SIGNED OFF] Bug 11096: add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Created attachment 24895 [details] [review] [SIGNED OFF] Bug 11096: Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 24896 [details] [review] Bug 11096: Koha<->Zebra should use the XML syntax with DOM This patch makes Koha <-> Zebra use MARCXML for the serialization when using DOM, and USMARC for GRS-1. * The following functions are modified to set the Zebra record syntax according to the current sysprefs and configuration: - C4::Context->Zconn - C4::Context-_new_Zconn * A new function 'new_record_from_zebra' is introduced, which checks the context we are in, and creates the MARC::Record object using the right constructor. The following packages get touched to make use of the new function: - C4::Search - C4::AuthoritiesMarc and the same happens to the UI scripts that make use of them (both in the OPAC and STAFF interfaces). * Calls to the unsafe ZOOM::Record->render()[1] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). * Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. * As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is correctly handled. * As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like: - bib_index_mode (defaults to 'grs1' if not specified) - auth_index_mode (defaults to 'dom') here we do exactly the same. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). - Test an index scan search (on the staff interface): Search > More options > Check "Scan indexes". Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba
Created attachment 24901 [details] [review] Bug 11096: Koha<->Zebra should use the XML syntax with DOM This patch makes Koha <-> Zebra use MARCXML for the serialization when using DOM, and USMARC for GRS-1. * The following functions are modified to set the Zebra record syntax according to the current sysprefs and configuration: - C4::Context->Zconn - C4::Context-_new_Zconn * A new function 'new_record_from_zebra' is introduced, which checks the context we are in, and creates the MARC::Record object using the right constructor. The following packages get touched to make use of the new function: - C4::Search - C4::AuthoritiesMarc and the same happens to the UI scripts that make use of them (both in the OPAC and STAFF interfaces). * Calls to the unsafe ZOOM::Record->render()[1] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). * Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. * As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is correctly handled. * As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like: - bib_index_mode (defaults to 'grs1' if not specified) - auth_index_mode (defaults to 'dom') here we do exactly the same. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). - Test an index scan search (on the staff interface): Search > More options > Check "Scan indexes". - Enable 'itemBarcodeFallbackSearch' and try to circulate any word, it shouldn't break. - Searching for a biblio in a new subscription shouldn't break. - Running bulkmarcimport.pl shouldn't break. - And so on... for the rest of the .pl files. Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba
Created attachment 25282 [details] [review] Bug 11096: Koha<->Zebra should use the XML syntax with DOM This patch makes Koha <-> Zebra use MARCXML for the serialization when using DOM, and USMARC for GRS-1. * The following functions are modified to set the Zebra record syntax according to the current sysprefs and configuration: - C4::Context->Zconn - C4::Context-_new_Zconn * A new function 'new_record_from_zebra' is introduced, which checks the context we are in, and creates the MARC::Record object using the right constructor. The following packages get touched to make use of the new function: - C4::Search - C4::AuthoritiesMarc and the same happens to the UI scripts that make use of them (both in the OPAC and STAFF interfaces). * Calls to the unsafe ZOOM::Record->render()[1] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). * Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. * As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is correctly handled. * As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like: - bib_index_mode (defaults to 'grs1' if not specified) - auth_index_mode (defaults to 'dom') here we do exactly the same. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). - Test an index scan search (on the staff interface): Search > More options > Check "Scan indexes". - Enable 'itemBarcodeFallbackSearch' and try to circulate any word, it shouldn't break. - Searching for a biblio in a new subscription shouldn't break. - Running bulkmarcimport.pl shouldn't break. - And so on... for the rest of the .pl files. Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 25634 [details] [review] [PASSED QA] Bug 11096: add direct regression tests This adds direct tests of indexing and retrieving bib records that are larger than the ISO2709 limit of 99,999 octets. To test: verify that prove -v t/db_dependent/Search.t passes. Signed-off-by: Galen Charlton <gmc@esilibrary.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 25635 [details] [review] [PASSED QA] Bug 11096: Wrap each test set configuration into a subtest This makes it easier to read. Also fixes the test count, and is rebased against 11499 which is needed to correctly test the patches for this bug (i.e. indexing mode needs to be correctly set in koha-conf.xml) Regards To+ Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 25636 [details] [review] [PASSED QA] Bug 11096: Koha<->Zebra should use the XML syntax with DOM This patch makes Koha <-> Zebra use MARCXML for the serialization when using DOM, and USMARC for GRS-1. * The following functions are modified to set the Zebra record syntax according to the current sysprefs and configuration: - C4::Context->Zconn - C4::Context-_new_Zconn * A new function 'new_record_from_zebra' is introduced, which checks the context we are in, and creates the MARC::Record object using the right constructor. The following packages get touched to make use of the new function: - C4::Search - C4::AuthoritiesMarc and the same happens to the UI scripts that make use of them (both in the OPAC and STAFF interfaces). * Calls to the unsafe ZOOM::Record->render()[1] method are removed. Due to this last change the code for building facets was rewritten. And for performance on the facets creation I pushed higher version dependencies for MARC::File::XML and MARC::Record (we rely on MARC::Field->as_string). * Calls to MARC::Record->new_from_xml and MARC::Record->new_from_usmarc are wrapped with eval for catching problems [2]. * As of bug 3087, UNIMARC uses the 'unimarc' record syntax. this case is correctly handled. * As of bug 7818 misc/migration_tools/rebuild_zebra.pl behaves like: - bib_index_mode (defaults to 'grs1' if not specified) - auth_index_mode (defaults to 'dom') here we do exactly the same. To test: - prove t/db_dependent/Search.t should pass. - Searching should remain functional. - Indexing and searching for a big record should work (that's what the unit tests do). - Test an index scan search (on the staff interface): Search > More options > Check "Scan indexes". - Enable 'itemBarcodeFallbackSearch' and try to circulate any word, it shouldn't break. - Searching for a biblio in a new subscription shouldn't break. - Running bulkmarcimport.pl shouldn't break. - And so on... for the rest of the .pl files. Regards To+ [1] http://search.cpan.org/~mirk/Net-Z3950-ZOOM/lib/ZOOM.pod#render() [2] a record that cannot be parsed by MARC::Record is simply skipped (bug 10684) Sponsored-by: Universidad Nacional de Cordoba Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Setting Passed QA: 1. Ran through the test plan without any failures using marc21 setup. - Tested authority searching still works as expected. - Tested bilbiographic searching still works as expected. - Simple search, also truncated search terms - Advanced search, also itemtype and pubyear limits - Tested dontmerge preference, editing a linked authority and confirming the change is made to the linked records as well 2. Ran db_dependant tests. Success all round 3. Ran through QA script. Success all round I don't have a working UNIMARC setup, so I've not tested this again unimarc, however, with Katrin's earlier sign-off climing unimarc was ok, I thought this OK to pass anyway.
Pushed to master. Thanks, Tomás!
Yay!
I hope this patch will not cause a lot of trouble for non-package users. At least I am struggling now with getting dom indexing to work in master. In 3.14.4 [without this patch], it worked right away. I would not recommend to push this to stable too quickly.
Hi Marcel, all my tests were done on a dev installation and it seemed to work ok there, but I didn't test the very last iteration I think. Can you give some more details about the problems you are facing?
(In reply to Katrin Fischer from comment #78) > Hi Marcel, all my tests were done on a dev installation and it seemed to > work ok there, but I didn't test the very last iteration I think. Can you > give some more details about the problems you are facing? OK. It seems to work fine. I was struggling with marcxml as element set name. But in a clean master it must work. I do not think that marcxml is an element set name (as we did use F for full in the old situation, which is more of an element set name, although we did not actually differentiate it from B [brief] at all). Just omitting the element set name here, could be even better. (I will submit some code in this direction on bug 9612 soon. May include it..) So no reasons for me anymore to postpone pushing this to stable.
What is the impact of this patch on search performances? My tests show that OPAC result page seems about 2 times slower with this patch when compared without.
(In reply to Frédéric Demians from comment #80) > What is the impact of this patch on search performances? My tests show that > OPAC result page seems about 2 times slower with this patch when compared > without. I noticed it also. I bet it comes from the change in facets code : "This patch also removes calls to the unsafe ZOOM::Record->render()" I agree this code is ugly but it was more performant.
(In reply to Fridolin SOMERS from comment #81) > I noticed it also. I bet it comes from the change in facets code : "This > patch also removes calls to the unsafe ZOOM::Record->render()" I agree this > code is ugly but it was more performant. Yes, the facets building code is the culprit. It can be seen easily by playing with maxRecordsForFacets syspref.
I think that we're having an issue with this in 3.14.5... We're still getting out of bounds errors for large records. Then, when we try to shorten those records, Zebra doesn't appear to be updating, even when doing a full re-index. Using MARCXML switch, DOM indexing, non-package install.
(In reply to David Cook from comment #83) > I think that we're having an issue with this in 3.14.5... > > We're still getting out of bounds errors for large records. > > Then, when we try to shorten those records, Zebra doesn't appear to be > updating, even when doing a full re-index. > > Using MARCXML switch, DOM indexing, non-package install. David, I don't think this has been pushed to the stable branches. Fridolin and Marcel, I'm filling a new bug for the performance hit. I think I have a patch to fix it.
(In reply to Tomás Cohen Arazi from comment #84) > (In reply to David Cook from comment #83) > > I think that we're having an issue with this in 3.14.5... > > > > We're still getting out of bounds errors for large records. > > > > Then, when we try to shorten those records, Zebra doesn't appear to be > > updating, even when doing a full re-index. > > > > Using MARCXML switch, DOM indexing, non-package install. > > David, I don't think this has been pushed to the stable branches. > > Fridolin and Marcel, I'm filling a new bug for the performance hit. I think > I have a patch to fix it. Ah, good one, Tomás. I was mixing up my dates. I thought "pushed to master" was for 3.14 but it was for 3.16. My bad!
Just a note that this should also solve a problem with long tag. For example 505 with long UTF-8 text will easily exceed ISO2709 standard (of 9,999 chars) and cause problem with Zebra index and display. Before this patch, for records with long tag, some index might work and some might not. Search display of these records may either missing (due to invalid UTF-8 sequence error), show up with no items or show up with broken link. (I haven't test that this patch solve the long tag problem. But theoritically, it should).
If marcxml + XSLT is to slow, then you might try to use turbomarc instead. It's a smaller XML for MARC and much faster to work with in XSLT.
(In reply to Wolfram Schneider from comment #87) > If marcxml + XSLT is to slow, then you might try to use turbomarc instead. > It's a smaller XML for MARC and much faster to work with in XSLT. http://www.indexdata.com/news/2014/06/turbomarc-faster-xml-marc-records I can't imagine us using Turbomarc for anything but indexing, but then we'd lose time converting from MARCXML to/from Turbomarc as well... Personally, I like the idea of using our own intermediary format which can be exported as MARCXML (or Turbomarc or whatever), although I suppose the problem with that is our intermediary format could grow and then not be perfectly exportable as MARCXML, which means lost data for users, although perhaps not a big problem if you provide easy ways of customizing exports, so people could export into fields like 9AA (see http://www.loc.gov/standards/marcxml/schema/MARC21slim.xsd for valid tag ranges) and then massage from there... The intermediary format could also help us move away from XML and Zebra anyway, so that we could use any indexing engine like Solr, ElasticSearch, or whatever. Really, we only need MARC(XML) for interchange. Consider this: "2.1. The MARC 21 formats are communication formats, primarily designed to provide specifications for the exchange of bibliographic and related information between systems. They are widely used in a variety of exchange and processing environments. As communication formats, they do not mandate internal storage or display formats to be used by individual systems." (https://www.loc.gov/marc/96principl.html) I suppose my argument could be used to support turbomarc as well... although I don't know how well turbomarc would work itself as a format (besides the performance improvement for XSL transformations). I suppose another argument against changing from MARC(XML) could be the need to provide sufficient mechanisms for using AACR2 and RDA. MARC pretty much already has that built in, but then it also has limitations. Anyway, that's all rather tangential. I don't know about turbomarc. It seems like it would be a bit painful to work with :/.