[Bug 20664] New: Optimize retrieval of biblio and item data
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Bug ID: 20664 Summary: Optimize retrieval of biblio and item data Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: ere.maijala@helsinki.fi QA Contact: testopia@bugs.koha-community.org There are several inefficiencies in the current process of retrieving biblios and embedding items in the MARC data. These affect especially batch processes such as exporting of records and the OAI-PMH provider. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |ere.maijala@helsinki.fi |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #1 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 74873 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=74873&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are three ways this is accomplished: 1.) Use state variables to hold prepared SQL statements so that the preparation is done only once. 2.) Create optimized method for fetching item fields for MARC embedding. 3.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #2 from Ere Maijala <ere.maijala@helsinki.fi> --- These changes improve performance also for other processes that call e.g. GetMarcBiblio repeatedly. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #3 from Ere Maijala <ere.maijala@helsinki.fi> --- My test results show and improvement from about 30 seconds to about 10 seconds with the example export in the test plan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Frédéric Demians <frederic@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |frederic@tamil.fr -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #74873|0 |1 is obsolete| | --- Comment #4 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 75968 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=75968&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are three ways this is accomplished: 1.) Use state variables to hold prepared SQL statements so that the preparation is done only once. 2.) Create optimized method for fetching item fields for MARC embedding. 3.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #5 from Ere Maijala <ere.maijala@helsinki.fi> --- Latest version fixes a mistake in the itemnumber check. Also rebased on master. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lari.taskula@jns.fi -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #6 from Lari Taskula <lari.taskula@jns.fi> --- Fails t/db_dependent/Items.t # Failed test 'For OPAC, the pref OpacHiddenItems should have been take into account. Only items with homebranch ne CPL should have been embedded' # at t/db_dependent/Items.t line 631. # got: '8' # expected: '2' # Failed test 'For OPAC, If all items are hidden, no item should have been embedded' # at t/db_dependent/Items.t line 642. # got: '8' # expected: '0' # Looks like you failed 2 tests of 7. # Failed test 'C4::Biblio::EmbedItemsInMarcBiblio' # at t/db_dependent/Items.t line 649. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. Use of uninitialized value in string ne at /home/koha/kohaclone/C4/Items.pm line 1729. # Failed test 'items.barcode is not mapped anymore, so the DB column has not been updated' # at t/db_dependent/Items.t line 726. # got: undef # expected: 'a barcode' # Looks like you failed 1 test of 4. t/db_dependent/Items.t .. 11/14 # Failed test 'C4::Items::_build_default_values_for_mod_marc' # at t/db_dependent/Items.t line 731. # Looks like you failed 2 tests of 14. t/db_dependent/Items.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/14 subtests Test Summary Report ------------------- t/db_dependent/Items.t (Wstat: 512 Tests: 14 Failed: 2) Failed tests: 10-11 Non-zero exit status: 2 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #7 from Lari Taskula <lari.taskula@jns.fi> --- (In reply to Lari Taskula from comment #6)
Fails t/db_dependent/Items.t This is due to caching parsed YAML from OpacHiddenItems. Cache does not cleared between tests when OpacHiddenItems changes.
Instead of additionally caching parsed YAML of this system preference, would it be better for Koha to handle YAML system preferences as parsed by default? This would require some changes to Koha core but I guess it would benefit all features using system preferences represented in YAML. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #8 from Ere Maijala <ere.maijala@helsinki.fi> --- Thanks for the feedback. I think I'll spin that off as a separate bug and remove the caching thing here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Lari Taskula <lari.taskula@jns.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20930 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #75968|0 |1 is obsolete| | --- Comment #9 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 76045 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76045&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are three ways this is accomplished: 1.) Use state variables to hold prepared SQL statements so that the preparation is done only once. 2.) Create optimized method for fetching item fields for MARC embedding. 3.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff --- Comment #10 from Ere Maijala <ere.maijala@helsinki.fi> --- Latest patch removes the custom YAML caching and fixes tests to properly flush all caches after changing the settings. Additionally fixes warnings surfaced by the use of Modern::Perl. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76045|0 |1 is obsolete| | --- Comment #11 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 76046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76046&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are three ways this is accomplished: 1.) Use state variables to hold prepared SQL statements so that the preparation is done only once. 2.) Create optimized method for fetching item fields for MARC embedding. 3.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76046|0 |1 is obsolete| | --- Comment #12 from Ere Maijala <ere.maijala@helsinki.fi> --- Comment on attachment 76046 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76046 Bug 20664: Optimize retrieval of biblio and item data I need to come up with a better mechanism for avoiding unnecessary query preparation since statements cached in state variables may become stale. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19365 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #13 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 76669 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=76669&action=edit Bug 20664: Optimize retrieval of biblio and item data https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two changes to accomplish this: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Also the now-unnecessary frameworkcode parameter to _strip_item_fields() has been removed. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #14 from Ere Maijala <ere.maijala@helsinki.fi> --- The latest patch is more simple and avoids any attempts at caching prepared statements. Fortunately MySQL prepares quickly, so the performance improvement is still good. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=21006 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #76669|0 |1 is obsolete| | --- Comment #15 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 78868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78868&action=edit Bug 20664: Optimize retrieval of biblio and item data https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two changes to accomplish this: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Also the now-unnecessary frameworkcode parameter to _strip_item_fields() has been removed. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #16 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 78869 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78869&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #17 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 78870 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78870&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #18 from Ere Maijala <ere.maijala@helsinki.fi> --- I shamelessly lifted Nick's tests from bug 21006. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #19 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 78868 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=78868 Bug 20664: Optimize retrieval of biblio and item data Review of attachment 78868: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20664&attachment=78868) ----------------------------------------------------------------- The changes to the hiding logic break extensions to OpacHiddenItems in bugs 14385 and eventually 10589. :( -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #20 from Ere Maijala <ere.maijala@helsinki.fi> --- Too bad, but it happens. Since bug 14385 is already signed off, I'll rework this. Looks like it will be a significant change... -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |14385 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14385 [Bug 14385] Extend OpacHiddenItems to allow specifying exempt borrower categories -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |josef.moravec@gmail.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |nick@bywatersolutions.com --- Comment #21 from Nick Clemens <nick@bywatersolutions.com> --- *** Bug 21006 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78868|0 |1 is obsolete| | --- Comment #22 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 79225 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79225&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: diff -u unoptimized.xml optimized.xml 6.) Run tests to verify that they still pass: prove t/db_dependent/Items.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #23 from Ere Maijala <ere.maijala@helsinki.fi> --- The latest version now incorporates the functionality added in bug 14385. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |ASSIGNED --- Comment #24 from Ere Maijala <ere.maijala@helsinki.fi> --- Oops, need to fix conflicts with tests. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79225|0 |1 is obsolete| | --- Comment #25 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 79227 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79227&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78869|0 |1 is obsolete| | --- Comment #26 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 79228 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79228&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #78870|0 |1 is obsolete| | --- Comment #27 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 79229 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79229&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79227|0 |1 is obsolete| | Attachment #79228|0 |1 is obsolete| | Attachment #79229|0 |1 is obsolete| | --- Comment #28 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79276 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79276&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #29 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79277 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79277&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #30 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79278 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79278&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #31 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 79279 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79279&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #32 from M. Tompsett <mtompset@hotmail.com> --- Comment on attachment 79276 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=79276 Bug 20664: Optimize retrieval of biblio and item data Review of attachment 79276: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20664&attachment=79276) ----------------------------------------------------------------- The differences between unimarc and marc21 and in the future an integration of both types of records in the same system might be a reason to leave the framework code parameter stuff in. If not, I just have a really bad feeling about removing the frameworkcode parameter. ::: C4/Biblio.pm @@ -310,4 @@
$frameworkcode = "" if !$frameworkcode || $frameworkcode eq "Default"; # XXX
- _strip_item_fields($record, $frameworkcode);
I really think removing the frameworkcode is a bad idea. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #33 from Ere Maijala <ere.maijala@helsinki.fi> --- (In reply to M. Tompsett from comment #32)
::: C4/Biblio.pm @@ -310,4 @@
$frameworkcode = "" if !$frameworkcode || $frameworkcode eq "Default"; # XXX
- _strip_item_fields($record, $frameworkcode);
I really think removing the frameworkcode is a bad idea.
The default framwork was made authoritative in bug 19096. As a result of that, the frameworkcode parameter in GetMarcFromKohaField function is no longer used. _strip_item_fields is still using GetMarcFromKohaField, it just doesn't need the frameworkcode for anything. So effectively the patch just removes an unused parameter. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #34 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Ere Maijala from comment #33)
(In reply to M. Tompsett from comment #32)
::: C4/Biblio.pm @@ -310,4 @@
$frameworkcode = "" if !$frameworkcode || $frameworkcode eq "Default"; # XXX
- _strip_item_fields($record, $frameworkcode);
I really think removing the frameworkcode is a bad idea.
The default framwork was made authoritative in bug 19096. As a result of that, the frameworkcode parameter in GetMarcFromKohaField function is no longer used. _strip_item_fields is still using GetMarcFromKohaField, it just doesn't need the frameworkcode for anything. So effectively the patch just removes an unused parameter.
Yeah still writing that patch :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Patch doesn't apply --- Comment #35 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- On top of 14385 as suggested: Applying: Bug 20664: Optimize retrieval of biblio and item data /usr/share/koha/devclone/.git/rebase-apply/patch:220: tab in indent. my ( $biblionumber, $itemnumbers, $hidingrules ) = @_; fatal: sha1 information is lacking or useless (t/db_dependent/Items.t). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Bug 20664 depends on bug 14385, which changed state. Bug 14385 Summary: Extend OpacHiddenItems to allow specifying exempt borrower categories https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14385 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to Master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #36 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- New year, new request for next steps.. be nice to get this moving again. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79276|0 |1 is obsolete| | --- Comment #37 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 84516 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84516&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79277|0 |1 is obsolete| | --- Comment #38 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 84517 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84517&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79278|0 |1 is obsolete| | --- Comment #39 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 84518 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84518&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #79279|0 |1 is obsolete| | --- Comment #40 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 84519 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84519&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #41 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 84520 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=84520&action=edit Bug 20664: (follow-up) Fix QA whitespace errors -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #42 from Ere Maijala <ere.maijala@helsinki.fi> --- Sorry for the delay. Now rebased and fixed the whitespace issues. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84516|0 |1 is obsolete| | --- Comment #43 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 85106 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85106&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84517|0 |1 is obsolete| | --- Comment #44 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 85107 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85107&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84518|0 |1 is obsolete| | --- Comment #45 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 85108 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85108&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84519|0 |1 is obsolete| | --- Comment #46 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 85109 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85109&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #84520|0 |1 is obsolete| | --- Comment #47 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 85110 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85110&action=edit Bug 20664: (follow-up) Fix QA whitespace errors Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #48 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- A nice little performance boost that I can't find any regressions in.. Passing QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@ptfs-europe |y.org |.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #49 from David Cook <dcook@prosentient.com.au> --- (In reply to Ere Maijala from comment #14)
The latest patch is more simple and avoids any attempts at caching prepared statements. Fortunately MySQL prepares quickly, so the performance improvement is still good.
I was really excited when I read that you were going to use cached prepared statements (as I've used them to gain huge performance boosts on other Perl projects), so I'm saddened to see this, but after reviewing GetMarcItemFields I see that it would be challenging/impossible to do well, because of the dynamic query generation with @itemnumbers. Was that the reason you didn't do it? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #50 from Ere Maijala <ere.maijala@helsinki.fi> ---
I was really excited when I read that you were going to use cached prepared statements (as I've used them to gain huge performance boosts on other Perl projects), so I'm saddened to see this, but after reviewing GetMarcItemFields I see that it would be challenging/impossible to do well, because of the dynamic query generation with @itemnumbers. Was that the reason you didn't do it?
Unfortunately cached prepared statements don't work well with Plack where the process can outlive the MySQL connection. We found that reconnection to MySQL would leave the prepared statements holding on to the previous connection leading to "MySQL server has gone away" errors. All this could be overcome with additional logic, but that'd be another effort. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #51 from David Cook <dcook@prosentient.com.au> --- (In reply to Ere Maijala from comment #50)
Unfortunately cached prepared statements don't work well with Plack where the process can outlive the MySQL connection. We found that reconnection to MySQL would leave the prepared statements holding on to the previous connection leading to "MySQL server has gone away" errors. All this could be overcome with additional logic, but that'd be another effort.
That's an interesting point, although I was thinking of just caching (actually, re-using is probably a more accurate word than caching here) the prepared statement in the scope of the batch operation. I wouldn't cache it at the level of the Plack web server. I actually had to deal with that reconnection issue on a separate project recently (see "validate-on-match" and "background-validation" at https://docs.jboss.org/jbossas/docs/Server_Configuration_Guide/beta500/html/...). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #52 from Ere Maijala <ere.maijala@helsinki.fi> --- Caching during a batch operation would be useful, but it's harder to do without messing the architecture. And there's still a chance that something hickups during a batch process and automatic reconnection doesn't kick in for the cached statement. In any case the performance benefit from caching the statements, while measurable and significant, isn't dramatic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #53 from David Cook <dcook@prosentient.com.au> --- (In reply to Ere Maijala from comment #52)
Caching during a batch operation would be useful, but it's harder to do without messing the architecture. And there's still a chance that something hickups during a batch process and automatic reconnection doesn't kick in for the cached statement. In any case the performance benefit from caching the statements, while measurable and significant, isn't dramatic.
Yeah, it would certainly require messing with the existing architecture, and I can understand not doing that. If something did hiccup during a batch process and automatic reconnection didn't kick in, then we'd trap and report the error, which doesn't seem like the end of the world to me. But no worries. That's interesting that it's not dramatic. I've seen dramatic performance benefits on other projects, although perhaps that was for more complex queries and higher volumes of data where fractions of seconds can add up dramatically. In any case, you don't have to justify it to me. I'm just curious. Thanks for explaining your thought process to me :). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #54 from David Cook <dcook@prosentient.com.au> --- Does this affect the export of biblios/items for Zebra indexing? I'm watching a rebuild_zebra.pl crawling along exporting the bib records, and thinking how surely it should be able to be faster than this. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #55 from Ere Maijala <ere.maijala@helsinki.fi> --- This should provide a nice performance improvement also for exports for indexing. MySQL is quick to prepare statements, that's why we can get away with it. Other databases may have different behavior, and there are certainly slower ones in that regard. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #56 from David Cook <dcook@prosentient.com.au> --- (In reply to Ere Maijala from comment #55)
This should provide a nice performance improvement also for exports for indexing.
Excellent.
MySQL is quick to prepare statements, that's why we can get away with it. Other databases may have different behavior, and there are certainly slower ones in that regard.
MySQL also makes some really bad query plans. Maybe that's how it achieves its speed. For Koha's user-generated SQL Reports I've actually resorted to using "index hints" lately to get efficient query plans. It's a MySQLism but can make a world of difference as well. In any case, I think any improvement is a well worthwhile improvement :D. So thanks for doing this one! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Patch doesn't apply --- Comment #57 from Nick Clemens <nick@bywatersolutions.com> --- Sorry, rebase needed here -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Patch doesn't apply |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85106|0 |1 is obsolete| | --- Comment #58 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85776 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85776&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85107|0 |1 is obsolete| | --- Comment #59 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85777 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85777&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85108|0 |1 is obsolete| | --- Comment #60 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85778 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85778&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85109|0 |1 is obsolete| | --- Comment #61 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85779 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85779&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85110|0 |1 is obsolete| | --- Comment #62 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85780 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85780&action=edit Bug 20664: (follow-up) Fix QA whitespace errors Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #63 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85781 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85781&action=edit Bug 20664: (follow-up) Fix tests on rebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #64 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- I am not convinced by the changes made to GetMarcFromKohaField. I would not cache anything here as GetMarcSubfieldStructure already cached it. In my test it does not bring any performances boost. I know we need improvement boost, but if it goes against consistency and code readability/maintainability I think we are not going to the right direction: => Use of raw SQL statements => Duplication of code (when we are trying to centralize it) - item type calculation Also it seems that tests are missing (`git grep UseHidingRulesWithBorrowerCategory t` does not return anything). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |ASSIGNED --- Comment #65 from Ere Maijala <ere.maijala@helsinki.fi> --- Caching in GetMarcFromKohaField is indeed useless, I'll remove it. Also the test missing is something I need to fix. I can also make the missing itype check use Koha::Item->effective_itemtype to avoid duplication since it shouldn't be a common situation. What I don't really like is to use a known slow method for fetching the data from the database. I can change it to use Koha::Items->search since the changes still help improve the performance, but I'm not convinced that's the way it should be. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #66 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Ere Maijala from comment #65)
Caching in GetMarcFromKohaField is indeed useless, I'll remove it. Also the test missing is something I need to fix. I can also make the missing itype check use Koha::Item->effective_itemtype to avoid duplication since it shouldn't be a common situation. What I don't really like is to use a known slow method for fetching the data from the database. I can change it to use Koha::Items->search since the changes still help improve the performance, but I'm not convinced that's the way it should be.
If you are calling Koha::Items->find( $item->{itemnumber} )->effective_itemtype, my guess is that the Koha::Items->search will not have a big impact. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #67 from Ere Maijala <ere.maijala@helsinki.fi> --- Yes, but originally all the item type checking could be avoided unless item-level_itypes was false or item didn't have itype. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85776|0 |1 is obsolete| | --- Comment #68 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85898 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85898&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85777|0 |1 is obsolete| | --- Comment #69 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85899 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85899&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85778|0 |1 is obsolete| | --- Comment #70 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85900 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85900&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85779|0 |1 is obsolete| | --- Comment #71 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85901 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85901&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85780|0 |1 is obsolete| | --- Comment #72 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85902 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85902&action=edit Bug 20664: (follow-up) Fix QA whitespace errors Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85781|0 |1 is obsolete| | --- Comment #73 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85903 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85903&action=edit Bug 20664: (follow-up) Fix tests on rebase -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #74 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85904 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85904&action=edit Bug 20664: (follow-up) Switch to Koha objects for retrieving items -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #75 from Ere Maijala <ere.maijala@helsinki.fi> --- Created attachment 85905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=85905&action=edit Bug 20664: (follow-up) Add tests for UseHidingRulesWithBorrowerCategory -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Josef Moravec <josef.moravec@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #85898|0 |1 is obsolete| | Attachment #85899|0 |1 is obsolete| | Attachment #85900|0 |1 is obsolete| | Attachment #85901|0 |1 is obsolete| | Attachment #85902|0 |1 is obsolete| | Attachment #85903|0 |1 is obsolete| | Attachment #85904|0 |1 is obsolete| | Attachment #85905|0 |1 is obsolete| | --- Comment #76 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86002&action=edit Bug 20664: Optimize retrieval of biblio and item data Optimizes embedding of item data in MARC and fixes several bottlenecks encountered while profiling OAI-PMH and exporting of records. There are two ways this is accomplished: 1.) Create optimized method for fetching item fields for MARC embedding. 2.) Use the cache service more and where repeated calls are made. Test plan: 1.) Before applying the patch, time an export_records.pl run for a set of biblios that also have items. Run it a couple of times to account for initial slowness and possible fluctuations. For example: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename unoptimized.xml 2.) Apply the patch. 3.) Time the export process again with a different output file: time misc/export_records.pl --record-type bibs --starting_biblionumber 960000 --ending_biblionumber 965000 --format xml --filename optimized.xml 4.) Verify that the optimized process is faster. 5.) Compare the resulting export files to make sure they're identical: Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #77 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86003 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86003&action=edit Bug 20664: Add unit tests for GetMarcItem To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #78 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86004 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86004&action=edit Bug 20664: Unit tests for GetMarcItemFields To test: prove -v t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #79 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86005 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86005&action=edit Bug 20664: (follow-up) Fix test for GetMarcItemFields Without this patch I got this error running the test: YAML Error: Stream does not end with newline character Test plan: prove t/db_dependent/Items.t Signed-off-by: Josef Moravec <josef.moravec@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #80 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86006 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86006&action=edit Bug 20664: (follow-up) Fix QA whitespace errors Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #81 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86007 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86007&action=edit Bug 20664: (follow-up) Fix tests on rebase Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #82 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86008 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86008&action=edit Bug 20664: (follow-up) Switch to Koha objects for retrieving items Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #83 from Josef Moravec <josef.moravec@gmail.com> --- Created attachment 86009 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86009&action=edit Bug 20664: (follow-up) Add tests for UseHidingRulesWithBorrowerCategory Signed-off-by: Josef Moravec <josef.moravec@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |In Discussion --- Comment #84 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- + # This is so much faster than using Koha::Items->search that it makes sense even if it's ugly. + my $query = 'SELECT * FROM items WHERE biblionumber = ?'; Patch still applies. If I am reading thru the comments above, the main point of discussion is now: Do we want to return to raw sql in the above lines? It is rather obvious that this is faster than Koha::Object/DBIx. But we made a choice for DBIx and are still wrestling to implement it in the codebase. What would be the decisive reason for making the exception here, and would it be a precedent for doing similar things elsewhere? Also note that although the test plan refers to verifying that things are faster, I do not see any benchmark figures on the report. Moving to discussion and sending mail to QA. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #85 from David Cook <dcook@prosentient.com.au> --- Comment on attachment 86002 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=86002 Bug 20664: Optimize retrieval of biblio and item data Review of attachment 86002: --> (https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20664&attachment=86002) ----------------------------------------------------------------- ::: C4/Items.pm @@ +1357,5 @@
+ my $item_level_itype = C4::Context->preference('item-level_itypes'); + # This is so much faster than using Koha::Items->search that it makes sense even if it's ugly. + my $query = 'SELECT * FROM items WHERE biblionumber = ?'; + if (@$itemnumbers) { + $query .= ' AND itemnumber IN (' . join(',', @$itemnumbers) . ')';
This should be adding ? placeholders and binding the itemnumbers before executing. While it would probably be rare, a malformed record could cause SQL errors here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #86 from David Cook <dcook@prosentient.com.au> --- (In reply to Marcel de Rooy from comment #84)
+ # This is so much faster than using Koha::Items->search that it makes sense even if it's ugly. + my $query = 'SELECT * FROM items WHERE biblionumber = ?';
Patch still applies. If I am reading thru the comments above, the main point of discussion is now: Do we want to return to raw sql in the above lines? It is rather obvious that this is faster than Koha::Object/DBIx. But we made a choice for DBIx and are still wrestling to implement it in the codebase. What would be the decisive reason for making the exception here, and would it be a precedent for doing similar things elsewhere? Also note that although the test plan refers to verifying that things are faster, I do not see any benchmark figures on the report.
Moving to discussion and sending mail to QA.
I'd say the decisive reason would be that the low-performance of DBIx::Class adds up when doing batch operations. While low-performance and high convenience might be tolerable for many single requests, if you're dealing with high volumes of data, it's a nightmare. Even fractions of a second per record can add up to unacceptably slow amounts of time. I've used raw SQL at https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10662#c221 for the same reason. I wish I had included benchmarks on that bug, but with the raw SQL and re-using the statement handle, I was able to process much higher volumes. I think it was like 100 records per second instead of 4 records per second. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #87 from Ere Maijala <ere.maijala@helsinki.fi> --- David, you're right about the placeholders for itemnumbers, though the change will only be necessary if the code will actually be used instead of Koha objects. About benchmarks, I find them difficult since everyone's mileage will vary. I can fabricate a worst case benchmark such as exporting 1000 biblios that each have 1000 items, but someone else would have hard time getting the impressive results I got. In many cases a simple situation with one item per biblio might not show a meaningful change at all. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #88 from David Cook <dcook@prosentient.com.au> --- Maybe this would be a compromise for Ere's situation: https://metacpan.org/pod/distribution/DBIx-Class/lib/DBIx/Class/Manual/Cookb... It wouldn't work for my scenario as I'm doing higher performance inserts, but this might be a nice way of adding arbitrary SQL while maintaining use of the DBIx::Class framework? (I was inspired by Class::DBI. I work on a legacy project that uses Class::DBI and it has some functionality for adding arbitrary SQL to the ORM: https://metacpan.org/pod/Class::DBI#Ima::DBI-queries) There's also some interesting discussion at https://www.perlmonks.org/?node_id=700283. I wonder if a person could define a custom search through https://metacpan.org/pod/DBIx::Class::ResultSet#ATTRIBUTES to achieve the performance that Ere wants. Ere, you're not re-using statement handles, right? So are you getting performance improvement basically by reducing the number of SQL queries being made? If so, you might be able to avoid the arbitrary SQL by just making a more detailed search as noted in the attributes link above. Worth thinking about maybe? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #89 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'd be interested to see exactly what Koha::Object search call you were making for comparison... I've always found the SQL::Abstract query compilation really pretty quick.. it's the result inflation into DBIx::Class (and then Koha::Object) objects that takes time.. This can be entirely skipped with DBIx::Class::ResultClass::HashRefInflator and friends.. that's the approach I would take personally I think. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 --- Comment #90 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Caveat.. if you're wanting related (JOINed) data you'll wan't to call 'prefetch' in the actual search call too... I'm not entirely sure how Koha::Objects plays games here as it just adds a further layer of complexity on top of DBIx::Class.. but I've certainly seen realy solid results when I've used ::HashRefInflator in other projects -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=19884 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23793 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20664 Ere Maijala <ere.maijala@helsinki.fi> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |WONTFIX Status|In Discussion |RESOLVED --- Comment #91 from Ere Maijala <ere.maijala@helsinki.fi> --- I won't pursue this further. There's nice progress in bug 23793 which makes embedding item data much cleaner. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org