[Bug 42877] New: Memoize _get_marc_mapping_rules to speed up record indexing
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 Bug ID: 42877 Summary: Memoize _get_marc_mapping_rules to speed up record indexing Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Searching - Elasticsearch Assignee: koha-bugs@lists.koha-community.org Reporter: martin.renvoize@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org Target Milestone: --- Koha::SearchEngine::Elasticsearch::_get_marc_mapping_rules() rebuilds the whole mapping ruleset (regex-parsing every search_marc_map entry) on every call to marc_records_to_documents, and is not memoized. For a bulk reindex (large batches) this is amortized over the batch. But for incremental indexing -- every AddBiblio/ModBiblio enqueues a background job that indexes a single record -- the full rebuild runs for every record. Profiling on 435 records (KTD, Elasticsearch): - _get_marc_mapping_rules alone: ~10.7 ms/call - per-record mapping otherwise: ~3.6 ms So rule rebuilding is roughly 75% of the CPU for a single-record index job. The parsed rules are deterministic per (marcflavour, index). Caching them, the same way _foreach_mapping already uses a 'state' cache, would make single-record indexing roughly 3.5x faster and remove the cost from every code path. Test plan: 1. prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t 2. Reindex a single biblio (or edit one) and confirm it still indexes correctly. 3. Benchmark single-record marc_records_to_documents before/after. -- 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=42877 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42881 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42881 [Bug 42881] Improve Elasticsearch Indexing performance -- 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=42877 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Status|NEW |ASSIGNED -- 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=42877 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff QA Contact|testopia@bugs.koha-communit |nick@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 --- Comment #1 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 200640 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=200640&action=edit Bug 42877: Memoize _get_marc_mapping_rules to speed up record indexing _get_marc_mapping_rules rebuilds a large rules hashref on every call by iterating all search field mappings and parsing MARC field expressions (~10.7 ms per call). marc_records_to_documents calls it once per call, so every single-record indexing job (each AddBiblio/ModBiblio enqueues a job that indexes one record) pays the full rebuild for that one record -- roughly 75% of that job's CPU. This caches the compiled rules at the package level, keyed by marcflavour and index (e.g. 'marc21.biblios'), built once per combination and reused. The existing '# TODO' in _get_marc_mapping_rules already noted this could be cached. Cache invalidation: clear_search_fields_cache() clears the rules cache. Like the existing mapping cache in _foreach_mapping (state %all_mappings), the rules cache is per-process and is not auto-invalidated when search mappings change at runtime -- clear_search_fields_cache must be called (as the mapping-edit paths already require, and a reindex is needed for mapping changes regardless). This patch therefore introduces no new staleness beyond what already exists. Test plan: 1. prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t 2. Confirm _get_marc_mapping_rules returns the cached reference on the second call, and that clear_search_fields_cache forces a rebuild. 3. Optionally benchmark a single-record index (edit a biblio) before/after. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 --- Comment #2 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208521 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208521&action=edit Benchmark scripts for full-reindex / batch indexing (bug 42881 family) Clarifying the goal of this work (bug 42881 family): the target is infrastructure-facing - the time and CPU a FULL REINDEX (misc/search_tools/rebuild_elasticsearch.pl) and LARGE BATCH re-indexes (batch record modification, imports; these enqueue index jobs of up to chunk_size = 5000 records) consume. It is not about how quickly a cataloguer sees one edit: in KTD we measured the edit-to-searchable time through the stock es_indexer_daemon and it is dominated by job dispatch (roughly one job per second), so these patches do not change it. I am attaching a small benchmark package (bug42881-es-indexing-bench.tar.gz, see README.txt inside) so testers can reproduce the before/after numbers below and, importantly, verify that the generated Elasticsearch documents are byte-for-byte identical (SHA-256 over every document) with and without the patch. Setup, once, in a KTD with Elasticsearch (stock sample data works; 10,000 generated records give steadier numbers - gen_records.pl + bulkmarcimport.pl, see README): mkdir -p _bench && cp <extracted files> _bench/ Test plan for this bug 1. prove t/db_dependent/Koha/SearchEngine/Elasticsearch.t (passes; includes the new "_get_marc_mapping_rules should cache results" subtest) 2. Before/after, with this patch applied on a branch: _bench/ab_bench.sh main HEAD Expect: "output identical to baseline" = yes at every batch size; DB statements per record drop by 1 at batch size 1. 3. Confirm the cache is cleared by clear_search_fields_cache (the same contract as the existing mapping cache): change a search field mapping (Administration > Search engine configuration), run misc/search_tools/rebuild_elasticsearch.pl -d -r -b, and check the new mapping is used (clear_search_fields_cache clears the rules cache). 4. Optional, full reindex wall-clock: bash _bench/bench_rebuild.sh before / after (see README). Our results (10,436 generated records, KTD + ES 9, one core, best of 3 interleaved rounds, spread between rounds 0-7%), decode + mapping, ms per record: batch 1: 24.8 -> 4.7 (5.3x) batch 10: 6.6 -> 4.6 (1.45x) batch 500: 4.6 -> 4.5 (1.02x) full rebuild_elasticsearch.pl: 103.8 s -> 102.6 s (1.01x) Honest expectation: this patch removes a ~10 ms per-call fixed cost, so it matters for many small calls (one-record index jobs) and is essentially neutral for a full reindex, which makes few, large calls. Its value for the infrastructure goal is in combination with 42878/42879 (together ~1.46x on a full rebuild, see bug 42881) and for deployments that index many small jobs. Please check the bulk numbers on your own hardware. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 --- Comment #3 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208526 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208526&action=edit Bug 42877: Memoize _get_marc_mapping_rules to speed up record indexing (rebased) Rebased onto current main; code unchanged from the previous patch. Re-uploaded as the first patch of a chain (42877 -> 42878 -> 42879 -> 42880) so the series applies cleanly in order. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #200640|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42877 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |42878 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=42878 [Bug 42878] Batch the available-item check in ES marc_records_to_documents -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org