|
Description
Ere Maijala
2021-04-30 06:53:04 UTC
I am not sure I understand. It's an iterator, we are not going to load all the records in memory. The max we expect is the --commit param passed to the script (default 5000). Waiting for your patch :) Created attachment 120329 [details] [review] Bug 28268: Improve memory usage when indexing authorities in Elasticsearch Retrieves the complete records one by one to avoid huge memory usage. Note that this removes the call to GuessAuthTypeCode, but it is done later in Koha::SearchEngine::Elasticsearch::marc_records_to_documents (and was never done if you asked to index a single record with --authid parameter). Test plan: 1. Apply patch 2. Reindex authorities: perl misc/search_tools/rebuild_elasticsearch.pl -a -d -v 3. Check that indexing completed successfully and results are correct. Created attachment 120330 [details] [review] Bug 28268: Improve memory usage when indexing authorities in Elasticsearch Retrieves the complete records one by one to avoid huge memory usage. Note that this removes the call to GuessAuthTypeCode, but it is done later in Koha::SearchEngine::Elasticsearch::marc_records_to_documents (and was never done if you asked to index a single record with --authid parameter). Test plan: 1. Apply patch 2. Reindex authorities: perl misc/search_tools/rebuild_elasticsearch.pl -a -d -v 3. Check that indexing completed successfully and results are correct. This patch changes the mechanism so that the callback used to retrieve a single record retrieves authtypecode and marcxml fields. Well, actually it delegates the work to get_from_authid. As a result the record set retrieved from database only contains authid, which consumes quite a bit less memory than all the fields together. This also mirrors the method used in get_all_biblios_iterator. Surprisingly this seems to be also faster than the old mechanism. Created attachment 120424 [details] [review] Bug 28268: Improve memory usage when indexing authorities in Elasticsearch Retrieves the complete records one by one to avoid huge memory usage. Note that this removes the call to GuessAuthTypeCode, but it is done later in Koha::SearchEngine::Elasticsearch::marc_records_to_documents (and was never done if you asked to index a single record with --authid parameter). Test plan: 1. Apply patch 2. Reindex authorities: perl misc/search_tools/rebuild_elasticsearch.pl -a -d -v 3. Check that indexing completed successfully and results are correct. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Created attachment 120865 [details]
performance comparison
Three runs before and after the patch, 53927 auth records - machine generated from about ~30k bibs
Command I ran:
/usr/bin/time -v perl misc/search_tools/rebuild_elasticsearch.pl -v -d -a
This does seem to decrease memory, but I found it tended to run a bit longer time wise The code is simple, I don't like the 'while(1)' Could the fetch of the next row be moved into the while? Nick, it's the same setup as in Biblio's iterator. If that's to be changed, I'd rather do it separately. Created attachment 120866 [details] [review] Bug 28268: Improve memory usage when indexing authorities in Elasticsearch Retrieves the complete records one by one to avoid huge memory usage. Note that this removes the call to GuessAuthTypeCode, but it is done later in Koha::SearchEngine::Elasticsearch::marc_records_to_documents (and was never done if you asked to index a single record with --authid parameter). Test plan: 1. Apply patch 2. Reindex authorities: perl misc/search_tools/rebuild_elasticsearch.pl -a -d -v 3. Check that indexing completed successfully and results are correct. Signed-off-by: Aleisha Amohia <aleishaamohia@hotmail.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.06 Doesn't apply cleanly to 20.05, please rebase if needed |