Bug 32250

Summary: link_bibs_to_authorities generates too many background jobs
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: MARC Authority data supportAssignee: Nick Clemens (kidclamp) <nick>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: david, dcook, emily.lamancusa, fridolin.somers, jonathan.druart, lucas
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.06
Attachments: Bug 32250: Linking authorities does not change holdability
Bug 32250: Only reindex records when commiting
Bug 32250: (follow-up) Commit background job to the DB
Bug 32250: Linking authorities does not change holdability
Bug 32250: Only reindex records when commiting
Bug 32250: (follow-up) Commit background job to the DB
Bug 32250: (follow-up) Remove use of AutoCommit=0
Bug 32250: (follow-up) Remove one more dbh commit and don't start a new transaction when done
Bug 32250: Linking authorities does not change holdability
Bug 32250: Only reindex records when commiting
Bug 32250: (follow-up) Commit background job to the DB
Bug 32250: (follow-up) Remove use of AutoCommit=0

Description Nick Clemens (kidclamp) 2022-11-17 14:45:32 UTC
When running the linker job we call ModBiblio for every linked record

We should pass skip_record_index and skip_holds_queue and capture the biblionumbers - we can then generate a job per commit, rather than per record
Comment 1 Nick Clemens (kidclamp) 2023-01-13 19:44:54 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
Comment 2 Nick Clemens (kidclamp) 2023-01-13 20:11:22 UTC
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
Comment 3 David Cook 2023-01-16 00:08:11 UTC
Looking good at a glance, and probably fixes some possible race conditions...
Comment 4 David Nind 2023-01-17 01:20:49 UTC
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
Comment 5 Nick Clemens (kidclamp) 2023-03-28 11:56:37 UTC
The last patch on bug 32594 will fix the ES job statuses
Comment 6 David Nind 2023-03-28 17:41:33 UTC
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).
Comment 7 Nick Clemens (kidclamp) 2023-04-20 15:55:47 UTC
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
Comment 8 Nick Clemens (kidclamp) 2023-04-21 13:39:17 UTC
Raising severity as the linker currently doesn't successfully index for Elasticsearch due to a race condition
Comment 9 David Nind 2023-04-22 21:47:24 UTC
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>
Comment 10 David Nind 2023-04-22 21:47:27 UTC
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>
Comment 11 David Nind 2023-04-22 21:47:30 UTC
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>
Comment 12 David Nind 2023-04-22 21:51:23 UTC
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)
Comment 13 Jonathan Druart 2023-04-25 14:11:34 UTC
I think we should remove AutoCommit here, see bug 27245.
Comment 14 Nick Clemens (kidclamp) 2023-04-26 12:51:04 UTC
Created attachment 150239 [details] [review]
Bug 32250: (follow-up) Remove use of AutoCommit=0

This patch moves to using txn_begin and txn_commit
Comment 15 Jonathan Druart 2023-04-27 05:12:15 UTC
What about this one?
  88 $dbh->commit();
Comment 16 Nick Clemens (kidclamp) 2023-04-27 12:39:25 UTC
Created attachment 150293 [details] [review]
Bug 32250: (follow-up) Remove one more dbh commit and don't start a new transaction when done
Comment 17 Jonathan Druart 2023-05-02 08:10:58 UTC
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>
Comment 18 Jonathan Druart 2023-05-02 08:11:01 UTC
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>
Comment 19 Jonathan Druart 2023-05-02 08:11:04 UTC
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>
Comment 20 Jonathan Druart 2023-05-02 08:11:07 UTC
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>
Comment 21 David Cook 2023-05-03 02:53:45 UTC
Just double checking... this is an Elasticsearch issue only right?
Comment 22 David Nind 2023-05-03 06:04:25 UTC
(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.
Comment 23 Tomás Cohen Arazi 2023-05-05 12:16:53 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 24 Martin Renvoize 2023-05-11 11:16:12 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 25 Lucas Gass 2023-05-15 22:17:21 UTC
This patchset does not apply cleanly to 22.05.x, no backport unless rebased.