Description
Nick Clemens (kidclamp)
2022-11-17 14:45:32 UTC
Created attachment 145305 [details] [review] Bug 32250: Linking authorities does not change holdability If the only change is a linked heading, we don't need to rebuild the holds queue To test: 1 - Set preference RealTimeHoldsQueue to enable 2 - Run link_bibs_to_authorities 3 - Note holds queue jobs generated 4 - Apply patch 5 - repeat 6 - No holds queue updates Created attachment 145319 [details] [review] Bug 32250: Only reindex records when commiting This patch adds an array to catch updated bibs, and defers indexing until a batch of changes is committed To test: 1 - Set LinkerModule system preference to either first or last match Alternate this between runs of the linker to ensure changes are made 2 - Set SearchEngine to Elasticsearch and reindex (to ensure index is built) 3 - perl misc/link_bibs_to_authorities.pl -v 4 - Check Admin->Jobs and see that many ES index jobs are queued 5 - Apply patch 6 - perl misc/link_bibs_to_authorities.pl -v 7 - Check Admin->Jobs and see that 1 index per commit from is enqueued Looking good at a glance, and probably fixes some possible race conditions... I've tested (using KTD with the sample data and ES7), and I think I have run through the test plan correctly. The changes in the first patch all seemed to work correctly - no more holds queue updates. For the second patch, for step 7, two jobs appear in the 'Current jobs only' list and don't disappear - even after leaving for about 10 minutes or so (I tried a couple of times, I also flushed the cache and ran restart_all). These are the entries in the current jobs table: 234 New null/1 Update Elasticsearch index 01/17/2023 01:16 233 New null/1 Update Elasticsearch index 01/17/2023 01:16 The last patch on bug 32594 will fix the ES job statuses I had another go at testing, but the jobs list doesn't seem to be uodating properly for me. Here is how I tested using KTD: 1. master is up-to-date, so the latest patches for bug 32594 are included. 2. Using ES7 with Searchengine set to Elasticsearch, rebuilt search index (koha-elasticsearch --rebuild -d -b -a kohadev). 3. Applied both patches, flush_memcached, restart_all. 4. Enabled RealTimeHoldsQueue system preference. 5. Ran perl misc/link_bibs_to_authorities.pl -v 6. Current job list look like this (no change after 10 minutes): 1 New null/1 Update Elasticsearch index 03/28/2023 17:18 2 New null/1 Update Elasticsearch index 03/28/2023 17:18 7. Changed LinkerModule system preference. 8. Ran perl misc/link_bibs_to_authorities.pl -v 9. There are now four jobs in the current job list: 1 New null/1 Update Elasticsearch index 03/28/2023 17:18 2 New null/1 Update Elasticsearch index 03/28/2023 17:18 4 New null/1 Update Elasticsearch index 03/28/2023 17:28 5 New null/1 Update Elasticsearch index 03/28/2023 17:28 Is there something else I need to do? It seems to me that either the ES index updates are not actually running, or that the status is not updating when it is finished (even though the latest patches for bug 32594 are in master). Created attachment 149969 [details] [review] Bug 32250: (follow-up) Commit background job to the DB We can't run a background job if it isn't in the database, however, this script runs with AutoCommit disabled. We need to enable it while generating the background job, then disable for the updates. I don't nkow if using a transaction would be preferable. I tried to solve this independently, but it requires consolidating the index requests to make this work easily Raising severity as the linker currently doesn't successfully index for Elasticsearch due to a race condition Created attachment 150100 [details] [review] Bug 32250: Linking authorities does not change holdability If the only change is a linked heading, we don't need to rebuild the holds queue To test: 1 - Set preference RealTimeHoldsQueue to enable 2 - Run link_bibs_to_authorities 3 - Note holds queue jobs generated 4 - Apply patch 5 - repeat 6 - No holds queue updates Signed-off-by: David Nind <david@davidnind.com> Created attachment 150101 [details] [review] Bug 32250: Only reindex records when commiting This patch adds an array to catch updated bibs, and defers indexing until a batch of changes is committed To test: 1 - Set LinkerModule system preference to either first or last match Alternate this between runs of the linker to ensure changes are made 2 - Set SearchEngine to Elasticsearch and reindex (to ensure index is built) 3 - perl misc/link_bibs_to_authorities.pl -v 4 - Check Admin->Jobs and see that many ES index jobs are queued 5 - Apply patch 6 - perl misc/link_bibs_to_authorities.pl -v 7 - Check Admin->Jobs and see that 1 index per commit from is enqueued Signed-off-by: David Nind <david@davidnind.com> Created attachment 150102 [details] [review] Bug 32250: (follow-up) Commit background job to the DB We can't run a background job if it isn't in the database, however, this script runs with AutoCommit disabled. We need to enable it while generating the background job, then disable for the updates. I don't nkow if using a transaction would be preferable. I tried to solve this independently, but it requires consolidating the index requests to make this work easily Signed-off-by: David Nind <david@davidnind.com> How I tested (using KTD): 1. Start KTD with Elastic search (ktd --es7 up). 2. Update bibliographic record number 73 to remove one of the 010 LCCN numbers. Otherwise there is an error when running perl misc/link_bibs_to_authorities.pl -v: C4::Biblio::_koha_modify_biblioitem_nonmarc(): DBI Exception: DBD::mysql::st execute failed: Data too long for column 'lccn' at row 1 at /kohadevbox/koha/C4/Biblio.pm line 432 3. Set the RealTimeHoldsQueue system preference to Enable. 4. Run the authorities linker: perl misc/link_bibs_to_authorities.pl -v 5. Check the jobs list (Administration > Jobs > Manage jobs). Note that holds queue jobs were generated. 6. Set the LinkerModule system preference to either first or last match - alternate this between runs of the linker to ensure changes are made. 7. Rebuild search index (koha-elasticsearch --rebuild -d -b -a kohadev). 8. Run the authorities linker: perl misc/link_bibs_to_authorities.pl -v 9. Check the jobs list (Administration > Jobs > Manage jobs). Note that many Elasticsearch index jobs are queued. 10. Apply the patches, flush_memcached, restart_all. 11. Change the Set LinkerModule system preference. 12. Run the authorities linker: perl misc/link_bibs_to_authorities.pl -v 13. Check the jobs list (Administration > Jobs > Manage jobs). Note that: - There are no hold queue updates - That are a smaller number of Elastic search update (3 for me, instead of an update for each record) I think we should remove AutoCommit here, see bug 27245. Created attachment 150239 [details] [review] Bug 32250: (follow-up) Remove use of AutoCommit=0 This patch moves to using txn_begin and txn_commit What about this one? 88 $dbh->commit(); Created attachment 150293 [details] [review] Bug 32250: (follow-up) Remove one more dbh commit and don't start a new transaction when done Created attachment 150501 [details] [review] Bug 32250: Linking authorities does not change holdability If the only change is a linked heading, we don't need to rebuild the holds queue To test: 1 - Set preference RealTimeHoldsQueue to enable 2 - Run link_bibs_to_authorities 3 - Note holds queue jobs generated 4 - Apply patch 5 - repeat 6 - No holds queue updates Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 150502 [details] [review] Bug 32250: Only reindex records when commiting This patch adds an array to catch updated bibs, and defers indexing until a batch of changes is committed To test: 1 - Set LinkerModule system preference to either first or last match Alternate this between runs of the linker to ensure changes are made 2 - Set SearchEngine to Elasticsearch and reindex (to ensure index is built) 3 - perl misc/link_bibs_to_authorities.pl -v 4 - Check Admin->Jobs and see that many ES index jobs are queued 5 - Apply patch 6 - perl misc/link_bibs_to_authorities.pl -v 7 - Check Admin->Jobs and see that 1 index per commit from is enqueued Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 150503 [details] [review] Bug 32250: (follow-up) Commit background job to the DB We can't run a background job if it isn't in the database, however, this script runs with AutoCommit disabled. We need to enable it while generating the background job, then disable for the updates. I don't nkow if using a transaction would be preferable. I tried to solve this independently, but it requires consolidating the index requests to make this work easily Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 150504 [details] [review] Bug 32250: (follow-up) Remove use of AutoCommit=0 This patch moves to using txn_begin and txn_commit Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Bug 32250: (follow-up) Remove one more dbh commit and don't start a new transaction when done Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Just double checking... this is an Elasticsearch issue only right? (In reply to David Cook from comment #21) > Just double checking... this is an Elasticsearch issue only right? Yes - see step 2 of the 'Only reindex records when commiting' patch. Pushed to master for 23.05. Nice work everyone, thanks! Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release This patchset does not apply cleanly to 22.05.x, no backport unless rebased. |