[Bug 42504] New: Remove totalissues from stored MARC, embed at index time
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Bug ID: 42504 Summary: Remove totalissues from stored MARC, embed at index time Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org ## Problem update_totalissues.pl can run for 4+ hours on large installations. The root cause is that totalissues is stored in the MARC record (942$0), requiring the full ModBiblio pipeline per biblio: MARC XML deserialization, field update, serialization, TransformMarcToKoha, DB updates, search engine reindexing, cataloguing log entries, plugin hooks, OAI sets updates. The existing UpdateTotalIssuesOnCirc syspref provides real-time updates on checkout but is disabled by default because it triggers this same expensive ModBiblio pipeline on every checkout. Bug 36474 improved things by skipping unchanged records, but every changed record still pays the full cost. ## Proposal Stop storing totalissues in the MARC record. Instead: 1. UpdateTotalIssues becomes a lightweight DB-only operation: just UPDATE biblioitems SET totalissues = ? WHERE biblionumber = ? plus queue a search index update. No MARC parsing, no ModBiblio. 2. Inject 942$0 at index time via Koha::Biblio::metadata_record: Add an embed_total_issues parameter (following the existing embed_items pattern). Both search engine indexers call metadata_record to get the MARC before indexing. The value gets injected from biblioitems.totalissues on the fly, just like items are embedded today. The stored MARC in biblio_metadata stays clean. Both the ES indexer (_get_record) and Zebra's rebuild_zebra.pl -d (which already pre-processes records before sending to Zebra) would pass this flag. 3. UpdateTotalIssuesOnCirc becomes cheap enough to enable by default: A DB column update plus a reindex queue entry per checkout is trivial. The Elasticsearch indexer already batches indexing tasks efficiently. 4. update_totalissues.pl is simplified: It recalculates biblioitems.totalissues from statistics/items data and triggers batch reindexing. It no longer touches MARC records at all. It should also gain a --commit option to control batch sizes for the reindex step. ## Expected impact - Real-time totalissues updates on checkout with negligible performance cost - update_totalissues.pl runtime reduced from hours to minutes (DB updates + batch reindex vs per-record ModBiblio) - No more MARC record churn for a derived counter value - Reduced action_logs noise (no cataloguing log entries for counter updates) -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch 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=42504 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198408 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198408&action=edit Bug 42504: Add embed_total_issues to metadata_record This patch adds an embed_total_issues parameter to Koha::Biblio::metadata_record that injects the current biblioitems.totalissues value into the MARC record, replacing any stale stored value. The target field is determined by the Koha-to-MARC mapping (942$0 for MARC21). Injection happens before the RecordProcessor so ViewPolicy visibility rules are respected. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Tests cover: - No injection without the flag - DB value correctly injected into MARC - Updated DB values reflected on subsequent calls - Stale MARC values replaced with current DB value 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198409&action=edit Bug 42504: Pass embed_total_issues to indexers Both the Elasticsearch indexer and rebuild_zebra.pl now pass embed_total_issues => 1 when calling metadata_record, so the search index always gets the current DB value for totalissues regardless of what is stored in the MARC record. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198410&action=edit Bug 42504: Simplify update_totalissues.pl The script no longer touches MARC records. It now: - Updates biblioitems.totalissues directly via SQL - Batch reindexes modified records via the search engine indexer - Pre-filters unchanged records at the SQL level (--use-items) - Adds --commit option to control reindex batch size (default: 1000) The full ModBiblio pipeline (MARC parse/serialize, TransformMarcToKoha, cataloguing log, plugin hooks, OAI sets) is completely bypassed. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ perl misc/cronjobs/update_totalissues.pl --use-items -v -t => SUCCESS: Script runs, reports processed/modified counts 3. Checkout an item, then run: k$ perl misc/cronjobs/update_totalissues.pl --use-items -v --commit=10 => SUCCESS: Only changed biblios updated, batch reindex triggered 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #4 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198411 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198411&action=edit Bug 42504: Move UpdateTotalIssues to Koha::Biblio Adds two methods to Koha::Biblio: - increment_total_issues: no params, increments by 1, triggers reindex. For real-time use on checkout. - calculate_total_issues({ use_items => 1 | use_stats => 1 }): recalculates from source data, updates DB, returns value. Testable encapsulation of the calculation logic. C4::Biblio::UpdateTotalIssues is kept as a thin wrapper for backward compatibility. C4::Circulation now calls $item_object->biblio->increment_total_issues directly. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Tests cover: - increment_total_issues: 0->1, 1->2, undef->1, returns $self - calculate_total_issues: use_items sum, use_stats count, DB updated, no-op when unchanged 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198412 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198412&action=edit Bug 42504: (follow-up) Remove C4::Biblio::UpdateTotalIssues The function is replaced by Koha::Biblio->increment_total_issues and Koha::Biblio->calculate_total_issues. No callers remain. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t => SUCCESS: Tests pass! 3. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198413 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198413&action=edit Bug 42504: (follow-up) Always increment total issues on checkout The UpdateTotalIssuesOnCirc syspref check is removed. The operation is now lightweight (DB update + reindex) so there is no reason to make it optional. Test plan: 1. Apply patch 2. Checkout an item 3. Verify biblioitems.totalissues was incremented => SUCCESS: Always updated regardless of syspref 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #7 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 198414 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=198414&action=edit Bug 42504: [OPTIONAL] Remove UpdateTotalIssuesOnCirc syspref Total issues are now always updated on checkout since the operation is lightweight. The syspref is no longer needed. Test plan: 1. Apply patch 2. Run: $ ktd --shell k$ perl installer/data/mysql/updatedatabase.pl => SUCCESS: Preference removed 3. Verify UpdateTotalIssuesOnCirc no longer appears in Administration > System preferences 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com | |, | |lucas@bywatersolutions.com, | |martin.renvoize@openfifth.c | |o.uk, | |nick@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=36013 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #8 from Fridolin Somers <fridolin.somers@biblibre.com> --- Note that with Elasticsearch there is already a search field 'issues' using 'sum' type. It is used for sorting by popularity. So total issues may be useless no ? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #9 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Fridolin Somers from comment #8)
Note that with Elasticsearch there is already a search field 'issues' using 'sum' type. It is used for sorting by popularity.
So total issues may be useless no ?
If you look at my last patch, I'm proposing to remove UpdateTotalIssuesOnCirc, which was introduced to avoid the overhead caused by updating MARC all the time. I think using the `issues` aggregate field is ok, but using the total issues is more accurate in terms of actual usage for the title: items can be deleted and stop counting towards the popularity. I personally like the pattern I come up with, even though I agree we could do it differently for Elastic. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jcamins@cpbibliography.com --- Comment #10 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- *** Bug 8115 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=42504 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=20418 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #11 from Fridolin Somers <fridolin.somers@biblibre.com> --- (In reply to Tomás Cohen Arazi (tcohen) from comment #9)
I think using the `issues` aggregate field is ok, but using the total issues is more accurate in terms of actual usage for the title: items can be deleted and stop counting towards the popularity.
Ah indeed, items can be deleted, so this data totalissues is completely valid ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 --- Comment #12 from David Nind <david@davidnind.com> --- Created attachment 199222 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=199222&action=edit Bug 42504: (follow-up) Increase number of tests Patch from commit de863f5 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42504 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #13 from David Nind <david@davidnind.com> --- I added a follow-up patch to increase the number of test from 45 to 46. Does this need to be tested with both Elasticsearch and Zebra? (I tested with es7.) I got these errors when running the commands from the "Simplify update_totalissues.pl" patch: perl misc/cronjobs/update_totalissues.pl --use-items -v -t main::process_query(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'biblioitems.totalissues' in 'HAVING' at misc/cronjobs/update_totalissues.pl line 124 perl misc/cronjobs/update_totalissues.pl --use-items -v --commit=10 main::process_query(): DBI Exception: DBD::mysql::st execute failed: Unknown column 'biblioitems.totalissues' in 'HAVING' at misc/cronjobs/update_totalissues.pl line 124 Also, not sure if this is related, but there are current jobs showing under Administration > Jobs > Manage jobs (after checking an item out): - Status = New - Progress: null/1 - Type: Update Elasticsearch index Consolidated testing notes so far (using KTD with es7): 1. Apply all the patches 2. Update the database: updatedatabase 3. Restart everything: restart_all 4. Run these tests, they should pass: prove t/Biblio.t t/db_dependent/Biblio.t t/db_dependent/Koha/Biblio.t 5. Verify that the UpdateTotalIssuesOnCirc system preference no longer exists. 6. Test the incrementing of the "Checkouts" column in the holdings table: 6.1 Checkout an item to a patron 6.2 For the item checked out in the holding table, show the "Checkouts" column, this should now be 1 6.3 Check the item in and check it out again to the same patron 6.4 The "Checkouts" column for the item should now be 2 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org