Currently any records that do not index simply fail silently To recreate: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated
Created attachment 89226 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output
Created attachment 89228 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output
I agree errors should be displayed, but I don't think we should fail indexing a record with an empty field.
(In reply to Ere Maijala from comment #3) > I agree errors should be displayed, but I don't think we should fail > indexing a record with an empty field. I don't know what I meant there, but I cannot recreate a record that will not index, at least not by simply adding an empty field. All sample records index successfully now. I don't think this patch is a bad idea still in the case that we do encounter errors
Agreed. Could you rebase, please?
Created attachment 95096 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elasticsearch.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elasticsearch.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elasticsearch.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elasticsearch.pl 14 - No output
Created attachment 95098 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi>
Created attachment 95530 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Do we really want to continue without adding tests?
(In reply to Nick Clemens from comment #0) > Currently any records that do not index simply fail silently > > To recreate: > 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest > way is using advanced cataloging editor > 2 - Reindex elasticsearch > 3 - Check the ES count on the about page > 4 - Check the count in the DB (SELECT count(*) FROM biblio) > 5 - They don't match! They match for me. Which record is supposed to be wrong in the sample data? I added a blank 245$b but the reindex is ok.
Created attachment 99040 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com>
Created attachment 99388 [details] [review] Bug 22828: Add tests
kohadev-koha@df2c0b4bf86c:/kohadevbox/koha$ perl misc/search_tools/rebuild_elasticsearch.pl -v [9614] Checking state of biblios index [9614] Checking state of authorities index [9614] Indexing biblios [9614] Committing final records... [9614] There were errors during indexing [9614] Total 436 records indexed [9614] Indexing authorities [9614] 1000 records processed [9614] Committing final records... [9614] Total 1706 records indexed with -v -v the output is not readable and the error is lost in the middle of the id. Should not we have it at the end?
Waiting for an answer.
(In reply to Jonathan Druart from comment #14) > Waiting for an answer. I cannot recreate an error at this time, so I am not sure what you mean. (In reply to Jonathan Druart from comment #13) > with -v -v the output is not readable and the error is lost in the middle of > the id. Should not we have it at the end? Can you show a sample and or what you propose? I don't think it actually matters where it falls. I expect in practice the output will be sent to a file. I am not saying there isn't room for improvement, but ensuring they are returned in any form is better than ignoring them as we do now
Let improve later then!
Created attachment 103645 [details] [review] Bug 22828: Elasticsearch - display errors encountered during indexing on the command line To test: 1 - Use the Koha sample data, or insert a blank 245$b into a record (easiest way is using advanced cataloging editor 2 - Reindex elasticsearch 3 - Check the ES count on the about page 4 - Check the count in the DB (SELECT count(*) FROM biblio) 5 - They don't match! 6 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 7 - No errors indicated 8 - Apply patch 9 - perl misc/search_tools/rebuild_elastic_search.pl -v 10 - You should be notified of an error 11 - perl misc/search_tools/rebuild_elastic_search.pl -v -v 12 - You should be notified of the specific biblio with an error and a (somewhat) readable reason 13 - perl misc/search_tools/rebuild_elastic_search.pl 14 - No output Signed-off-by: Ere Maijala <ere.maijala@helsinki.fi> Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr> Signed-off-by: Bouzid Fergani <bouzid.fergani@inlibro.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 103646 [details] [review] Bug 22828: Add tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
Backported to 19.11.x for 19.11.06
backported to 19.05.x for 19.05.11