Bug 22690 - Merging records with many items too slow (Elasticsearch)
Summary: Merging records with many items too slow (Elasticsearch)
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: Searching - Elasticsearch (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Ere Maijala
QA Contact:
URL:
Keywords:
Depends on:
Blocks: 20447
  Show dependency treegraph
 
Reported: 2019-04-11 14:19 UTC by Vitor Fernandes
Modified: 2019-10-22 07:09 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 22690 - Merging records with many items (ElasticSearch) (5.36 KB, patch)
2019-05-24 15:27 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22690: Merging records with many items (ElasticSearch) (5.16 KB, patch)
2019-05-27 11:28 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22690: Merging records with many items (ElasticSearch) (5.12 KB, patch)
2019-05-27 11:34 UTC, axel Amghar
Details | Diff | Splinter Review
Bug 22690: Refactor merging of records to improve performance (Elasticsearch) (16.57 KB, patch)
2019-09-16 11:16 UTC, Ere Maijala
Details | Diff | Splinter Review
Bug 22690: Refactor merging of records to improve performance (Elasticsearch) (16.23 KB, patch)
2019-10-22 07:09 UTC, Ere Maijala
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Vitor Fernandes 2019-04-11 14:19:26 UTC
When merging records with many items, Koha reindexes the origin record and the destination record for each item moving (check MoveItemFromBiblio in C4::Items).
If the record being eliminated has 1000 items, Koha will make 2000 reindexes.
Because of this behaviour, the merging is too slow.

Test plan:

- Create 2 records (one with 1000 items for example)
- Add the records to one list and start the merging process of those records. 
- Choose the record with many items as the one to be eliminated.
- Start the merging
- After a while the web server should give you a timeout error (the merging process continues)
Comment 1 axel Amghar 2019-05-24 07:36:59 UTC
hello,
when i try to merge records, i have an issue :
 Following required fields are missing:
-801


https://snag.gy/76Fh9R.jpg
this is a mandatory tag but i can't add it 
Do someone know why?
Comment 2 Vitor Fernandes 2019-05-24 09:51:17 UTC
(In reply to axel Amghar from comment #1)
> hello,
> when i try to merge records, i have an issue :
>  Following required fields are missing:
> -801
> 
> 
> https://snag.gy/76Fh9R.jpg
> this is a mandatory tag but i can't add it 
> Do someone know why?

Axel that happens because the framework chosen for merging has 801 as mandatory field (that's my guess).
Comment 3 axel Amghar 2019-05-24 10:01:53 UTC
(In reply to Vitor Fernandes from comment #2)

> Axel that happens because the framework chosen for merging has 801 as
> mandatory field (that's my guess).

I think is the same thing with "000" and "001".
They are mandatory but we can't add them.
So..
Do we have to fix it ?
Comment 4 axel Amghar 2019-05-24 15:27:57 UTC Comment hidden (obsolete)
Comment 5 axel Amghar 2019-05-27 11:28:45 UTC Comment hidden (obsolete)
Comment 6 axel Amghar 2019-05-27 11:34:40 UTC
Created attachment 90116 [details] [review]
Bug 22690: Merging records with many items (ElasticSearch)

This patch allow us to merged many items without timeout

Test plan :

Without the patch :

> - Create 2 records (one with 1000 items for example)
> - Add the records to one list and start the merging process of those records.
> - Choose the record with many items as the one to be eliminated.
> - Start the merging
> - After a while the web server should give you a timeout error (the merging process continues)

With the patch :
- make the same
- this time verify that all items have been merged without time out
Comment 7 axel Amghar 2019-05-27 11:42:08 UTC
Hello,
I found an issue when you click on merge selected, and then you click instant on merge,
the destination record doesn't have the time to load.
And when you look your record, it lose his MARC record.
Someone else succeed to reproduce this bug?
Comment 8 Ere Maijala 2019-07-29 08:28:04 UTC
I don't get the logic of the $items_number parameter. As far as I can see the first UPDATE moves all items regardless of the parameter, but the subsequent processing of acquisitions etc. only goes through items in the $items_number parameter. If MoveItemsFromBiblio is supposed to move all items, I'd remove the $items_number parameter and do everything necessary inside MoveItemsFromBiblio.

Also, I believe the 801 issue should be handled in a separate bug since it's not related to Elasticsearch.
Comment 9 Ere Maijala 2019-09-12 07:14:38 UTC
Taking this since I need this for bug 20447.
Comment 10 Ere Maijala 2019-09-16 11:16:10 UTC
Created attachment 92828 [details] [review]
Bug 22690: Refactor merging of records to improve performance (Elasticsearch)

This patch allows merging of records with many items without the web server timing out.

Test plan:

Without the patch:

- Create 2 records (one with e.g. 1000 items).
- Do a cataloguing search that displays both records, select them and click "Merge selected".
- Choose the record with many items as the one to be eliminated.
- Start the merging.
- After a while the web server should give you a timeout error (the merging process may still continue)

With the patch:
- Do the same as above
- This time verify that the records are merged without timeout
- Create a new biblio with an item
- Add with the item:
  * acquisition order
  * hold (reserve)
- Merge the biblio to another one
- Verify that the item and its related data was moved
- Verify that tests pass:
  prove -v t/db_dependent/Koha/Item.t
  prove -v t/db_dependent/Items/MoveItemFromBiblio.t
Comment 11 Ere Maijala 2019-10-22 07:09:58 UTC
Created attachment 94505 [details] [review]
Bug 22690: Refactor merging of records to improve performance (Elasticsearch)

This patch allows merging of records with many items without the web server timing out.

Test plan:

Without the patch:

- Create 2 records (one with e.g. 1000 items).
- Do a cataloguing search that displays both records, select them and click "Merge selected".
- Choose the record with many items as the one to be eliminated.
- Start the merging.
- After a while the web server should give you a timeout error (the merging process may still continue)

With the patch:
- Do the same as above
- This time verify that the records are merged without timeout
- Create a new biblio with an item
- Add with the item:
  * acquisition order
  * hold (reserve)
- Merge the biblio to another one
- Verify that the item and its related data was moved
- Verify that tests pass:
  prove -v t/db_dependent/Koha/Item.t
  prove -v t/db_dependent/Items/MoveItemFromBiblio.t