Bugzilla – Attachment 162015 Details for
Bug 35345
Pass custom SQL to rebuild_elasticsearch.pl to determine which records to index
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 35345: Add --where option to rebuild_elasticsearch.pl
Bug-35345-Add---where-option-to-rebuildelasticsear.patch (text/plain), 21.40 KB, created by
David Nind
on 2024-02-12 02:03:43 UTC
(
hide
)
Description:
Bug 35345: Add --where option to rebuild_elasticsearch.pl
Filename:
MIME Type:
Creator:
David Nind
Created:
2024-02-12 02:03:43 UTC
Size:
21.40 KB
patch
obsolete
>From de77ed1fd44ebfee485ce255b5b8f758b3079755 Mon Sep 17 00:00:00 2001 >From: Thomas Klausner <domm@plix.at> >Date: Wed, 15 Nov 2023 22:16:23 +0100 >Subject: [PATCH] Bug 35345: Add --where option to rebuild_elasticsearch.pl >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >Sometimes we need to only re-index a subset of our bibliographic data or authorities. Currently this is only possible by enumerating all id (-bn or -ai), which does not work well when indexing eg 100.000 items of a 2.000.000 DB. Re-indexing everything is also overkill. > >This patch adds an `--where` flag to misc/search_tools/rebuild_elasticsearch.pl which can take arbitrary SQL (that of course has to match the respective tables) and adds it as an additional param to the resultset to index > >To test, start koha-testing-docker with ElasticSearch enabled, for example via `ktd --es7 up > >Before applying the patch, rebuild_elasticsearch will index all data: > >Biblios: >$ misc/search_tools/rebuild_elasticsearch.pl -b -v >[12387] Checking state of biblios index >[12387] Indexing biblios >[12387] Committing final records... >[12387] Total 435 records indexed >(there might be a waring regarding a broken biblio, which can be ignored) > >Auth: >$ misc/search_tools/rebuild_elasticsearch.pl -a -v >[12546] Checking state of authorities index >[12546] Indexing authorities >[12546] 1000 records processed >[12546] Committing final records... >[12546] Total 1706 records indexed > >Now apply the patch > >Biblio, limit by range of biblioid: >$ misc/search_tools/rebuild_elasticsearch.pl -b -v --where "biblionumber between 100 and 150" >[12765] Checking state of biblios index >[12765] Indexing biblios >[12765] Committing final records... >[12765] Total 50 records indexed > >Note that only 50 records where indexed (instead of the whole set of 435 records) > >Auth, limit by authtypecode: >$ misc/search_tools/rebuild_elasticsearch.pl -a -v --where "authtypecode = 'GEOGR_NAME'" >[12848] Checking state of authorities index >[12848] Indexing authorities >[12848] Committing final records... >[12848] Total 142 records indexed > >Again, only 142 have been indexed. > >Note that this patch also includes perltidied versions of the affected files (because the qa script complained) > >Sponsored-by: Steiermärkische Landesbibliothek >Signed-off-by: David Nind <david@davidnind.com> >--- > Koha/BiblioUtils.pm | 43 +++++---- > Koha/MetadataRecord/Authority.pm | 99 +++++++++++-------- > misc/search_tools/rebuild_elasticsearch.pl | 105 ++++++++++++--------- > 3 files changed, 148 insertions(+), 99 deletions(-) > >diff --git a/Koha/BiblioUtils.pm b/Koha/BiblioUtils.pm >index cb7baed0db..d28dbf7db3 100644 >--- a/Koha/BiblioUtils.pm >+++ b/Koha/BiblioUtils.pm >@@ -34,7 +34,6 @@ use Koha::MetadataIterator; > use Koha::Database; > use Modern::Perl; > >- > use base qw(Koha::MetadataRecord); > > __PACKAGE__->mk_accessors(qw( record schema id datatype )); >@@ -51,8 +50,8 @@ the biblionumber can be provided too. > =cut > > sub new { >- my $class = shift; >- my $record = shift; >+ my $class = shift; >+ my $record = shift; > my $biblionumber = shift; > > my $self = $class->SUPER::new( >@@ -89,10 +88,10 @@ It will return C<undef> if the biblio doesn't exist. > =cut > > sub get_from_biblionumber { >- my ($class, $bibnum, %options) = @_; >+ my ( $class, $bibnum, %options ) = @_; > >- my $marc = $class->get_marc_biblio($bibnum, %options); >- return $class->new($marc, $bibnum); >+ my $marc = $class->get_marc_biblio( $bibnum, %options ); >+ return $class->new( $marc, $bibnum ); > } > > =head2 get_all_biblios_iterator >@@ -120,34 +119,41 @@ records instead of all. > =cut > > sub get_all_biblios_iterator { >- my ($class, %options) = @_; >+ my ( $class, %options ) = @_; > > my $search_terms = {}; >- my ($slice_modulo, $slice_count); >- if ($options{slice}) { >- $slice_count = $options{slice}->{count}; >+ my ( $slice_modulo, $slice_count ); >+ if ( $options{slice} ) { >+ $slice_count = $options{slice}->{count}; > $slice_modulo = $options{slice}->{index}; > $search_terms = \[ 'mod(biblionumber, ?) = ?', $slice_count, $slice_modulo ]; > } > > my $search_options = { columns => [qw/ biblionumber /] }; >- if ( $options{desc} ){ >- $search_options->{order_by} = { -desc => 'biblionumber' }; >+ if ( $options{desc} ) { >+ $search_options->{order_by} = { -desc => 'biblionumber' }; > } > > my $database = Koha::Database->new(); > my $schema = $database->schema(); >- my $rs = Koha::Biblios->search( >+ my $rs = Koha::Biblios->search( > $search_terms, >- $search_options ); >+ $search_options >+ ); >+ >+ if ( my $sql = $options{where} ) { >+ $rs = $rs->search( \[$sql] ); >+ } >+ > my $next_func = sub { >+ > # Warn and skip bad records, otherwise we break the loop > while (1) { > my $row = $rs->next(); > return if !$row; > my $next = eval { >- my $marc = $row->metadata->record({ embed_items => 1 }); >- $class->new($marc, $row->biblionumber); >+ my $marc = $row->metadata->record( { embed_items => 1 } ); >+ $class->new( $marc, $row->biblionumber ); > }; > if ($@) { > warn sprintf "Something went wrong reading record for biblio %s: %s\n", $row->biblionumber, $@; >@@ -180,10 +186,9 @@ If set to true, item data is embedded in the record. Default is to not do this. > =cut > > sub get_marc_biblio { >- my ($class, $bibnum, %options) = @_; >+ my ( $class, $bibnum, %options ) = @_; > >- my $record = Koha::Biblios->find($bibnum) >- ->metadata->record( { $options{item_data} ? ( embed_items => 1 ) : () } ); >+ my $record = Koha::Biblios->find($bibnum)->metadata->record( { $options{item_data} ? ( embed_items => 1 ) : () } ); > return $record; > } > >diff --git a/Koha/MetadataRecord/Authority.pm b/Koha/MetadataRecord/Authority.pm >index d16970a828..e9a8466c43 100644 >--- a/Koha/MetadataRecord/Authority.pm >+++ b/Koha/MetadataRecord/Authority.pm >@@ -67,7 +67,6 @@ sub new { > return $self; > } > >- > =head2 get_from_authid > > my $auth = Koha::MetadataRecord::Authority->get_from_authid($authid); >@@ -80,23 +79,35 @@ unfortunate but unavoidable fact. > =cut > > sub get_from_authid { >- my $class = shift; >- my $authid = shift; >+ my $class = shift; >+ my $authid = shift; > my $marcflavour = lc C4::Context->preference("marcflavour"); > >- my $dbh=C4::Context->dbh; >- my $sth=$dbh->prepare("select authtypecode, marcxml from auth_header where authid=?"); >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare("select authtypecode, marcxml from auth_header where authid=?"); > $sth->execute($authid); >- my ($authtypecode, $marcxml) = $sth->fetchrow; >- my $record=eval {MARC::Record->new_from_xml(StripNonXmlChars($marcxml),'UTF-8', >- (C4::Context->preference("marcflavour") eq "UNIMARC"?"UNIMARCAUTH":C4::Context->preference("marcflavour")))}; >+ my ( $authtypecode, $marcxml ) = $sth->fetchrow; >+ my $record = eval { >+ MARC::Record->new_from_xml( >+ StripNonXmlChars($marcxml), 'UTF-8', >+ ( >+ C4::Context->preference("marcflavour") eq "UNIMARC" >+ ? "UNIMARCAUTH" >+ : C4::Context->preference("marcflavour") >+ ) >+ ); >+ }; > return if ($@); > $record->encoding('UTF-8'); > >- my $self = $class->SUPER::new( { authid => $authid, >- authtypecode => $authtypecode, >- schema => $marcflavour, >- record => $record }); >+ my $self = $class->SUPER::new( >+ { >+ authid => $authid, >+ authtypecode => $authtypecode, >+ schema => $marcflavour, >+ record => $record >+ } >+ ); > > bless $self, $class; > return $self; >@@ -111,16 +122,24 @@ Create the Koha::MetadataRecord::Authority object associated with the provided a > =cut > > sub get_from_breeding { >- my $class = shift; >+ my $class = shift; > my $import_record_id = shift; >- my $marcflavour = lc C4::Context->preference("marcflavour"); >+ my $marcflavour = lc C4::Context->preference("marcflavour"); > >- my $dbh=C4::Context->dbh; >- my $sth=$dbh->prepare("select marcxml from import_records where import_record_id=? and record_type='auth';"); >+ my $dbh = C4::Context->dbh; >+ my $sth = $dbh->prepare("select marcxml from import_records where import_record_id=? and record_type='auth';"); > $sth->execute($import_record_id); > my $marcxml = $sth->fetchrow; >- my $record=eval {MARC::Record->new_from_xml(StripNonXmlChars($marcxml),'UTF-8', >- (C4::Context->preference("marcflavour") eq "UNIMARC"?"UNIMARCAUTH":C4::Context->preference("marcflavour")))}; >+ my $record = eval { >+ MARC::Record->new_from_xml( >+ StripNonXmlChars($marcxml), 'UTF-8', >+ ( >+ C4::Context->preference("marcflavour") eq "UNIMARC" >+ ? "UNIMARCAUTH" >+ : C4::Context->preference("marcflavour") >+ ) >+ ); >+ }; > return if ($@); > $record->encoding('UTF-8'); > >@@ -131,10 +150,13 @@ sub get_from_breeding { > require C4::AuthoritiesMarc; > my $authtypecode = C4::AuthoritiesMarc::GuessAuthTypeCode($record); > >- my $self = $class->SUPER::new( { >- schema => $marcflavour, >- authtypecode => $authtypecode, >- record => $record }); >+ my $self = $class->SUPER::new( >+ { >+ schema => $marcflavour, >+ authtypecode => $authtypecode, >+ record => $record >+ } >+ ); > > bless $self, $class; > return $self; >@@ -142,8 +164,8 @@ sub get_from_breeding { > > sub authorized_heading { > my ($self) = @_; >- if ($self->schema =~ m/marc/) { >- return Koha::Util::MARC::getAuthorityAuthorizedHeading($self->record, $self->schema); >+ if ( $self->schema =~ m/marc/ ) { >+ return Koha::Util::MARC::getAuthorityAuthorizedHeading( $self->record, $self->schema ); > } > return; > } >@@ -173,14 +195,12 @@ records instead of all. > =cut > > sub get_all_authorities_iterator { >- my ($self, %options) = @_; >+ my ( $self, %options ) = @_; > >- my $search_terms = { >- marcxml => { '!=', undef } >- }; >- my ($slice_modulo, $slice_count); >- if ($options{slice}) { >- $slice_count = $options{slice}->{count}; >+ my $search_terms = { marcxml => { '!=', undef } }; >+ my ( $slice_modulo, $slice_count ); >+ if ( $options{slice} ) { >+ $slice_count = $options{slice}->{count}; > $slice_modulo = $options{slice}->{index}; > $search_terms = { > '-and' => [ >@@ -191,24 +211,29 @@ sub get_all_authorities_iterator { > } > > my $search_options->{columns} = [qw/ authid /]; >- if ($options{desc}) { >+ if ( $options{desc} ) { > $search_options->{order_by} = { -desc => 'authid' }; > } > > my $database = Koha::Database->new(); > my $schema = $database->schema(); >- my $rs = >- $schema->resultset('AuthHeader')->search( >+ my $rs = $schema->resultset('AuthHeader')->search( > $search_terms, >- $search_options); >+ $search_options >+ ); >+ >+ if ( my $sql = $options{where} ) { >+ $rs = $rs->search( \[$sql] ); >+ } > my $next_func = sub { >+ > # Warn and skip bad records, otherwise we break the loop > while (1) { > my $row = $rs->next(); > return if !$row; > >- my $auth = __PACKAGE__->get_from_authid($row->authid); >- if (!$auth) { >+ my $auth = __PACKAGE__->get_from_authid( $row->authid ); >+ if ( !$auth ) { > warn "Something went wrong reading record for authority $row->authid: $@\n"; > next; > } >diff --git a/misc/search_tools/rebuild_elasticsearch.pl b/misc/search_tools/rebuild_elasticsearch.pl >index 337ac19dd2..ffb7301238 100755 >--- a/misc/search_tools/rebuild_elasticsearch.pl >+++ b/misc/search_tools/rebuild_elasticsearch.pl >@@ -34,6 +34,7 @@ B<rebuild_elasticsearch.pl> > [B<--desc>] > [B<-bn|--bnumber>] > [B<-ai|--authid>] >+[B<-w|--where SQL>] > [B<-p|--processes>] > [B<-v|--verbose>] > [B<-h|--help>] >@@ -87,6 +88,10 @@ repeated. > Only index the supplied authority id, mostly for testing purposes. May be > repeated. > >+=item B<-w|--where> >+ >+Pass some additional SQL to limit the records to be indexed. >+ > =item B<-p|--processes> > > Number of processes to use for indexing. This can be used to do more indexing >@@ -125,13 +130,13 @@ use Pod::Usage qw( pod2usage ); > use Try::Tiny qw( catch try ); > > my $verbose = 0; >-my $commit = 5000; >-my ($delete, $reset, $help, $man, $processes); >-my ($index_biblios, $index_authorities); >-my (@biblionumbers,@authids); >+my $commit = 5000; >+my ( $delete, $reset, $help, $man, $processes ); >+my ( $index_biblios, $index_authorities ); >+my ( @biblionumbers, @authids, $where ); > my $desc; > >-$|=1; # flushes output >+$| = 1; # flushes output > > GetOptions( > 'c|commit=i' => \$commit, >@@ -142,6 +147,7 @@ GetOptions( > 'desc' => \$desc, > 'bn|bnumber=i' => \@biblionumbers, > 'ai|authid=i' => \@authids, >+ 'w|where=s' => \$where, > 'p|processes=i' => \$processes, > 'v|verbose+' => \$verbose, > 'h|help' => \$help, >@@ -149,62 +155,69 @@ GetOptions( > ); > > # Default is to do both >-unless ($index_authorities || $index_biblios) { >+unless ( $index_authorities || $index_biblios ) { > $index_authorities = $index_biblios = 1; > } > >-if ($processes && ( @biblionumbers || @authids) ) { >+if ( $processes && ( @biblionumbers || @authids ) ) { > die "Argument p|processes cannot be combined with bn|bnumber or ai|authid"; > } > >-pod2usage(1) if $help; >+pod2usage(1) if $help; > pod2usage( -exitstatus => 0, -verbose => 2 ) if $man; > > _sanity_check(); > >-if ($reset){ >+if ($reset) { > Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings; > $delete = 1; > } > >-_verify_index_state($Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX, $delete) if ($index_biblios); >-_verify_index_state($Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX, $delete) if ($index_authorities); >+_verify_index_state( $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX, $delete ) if ($index_biblios); >+_verify_index_state( $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX, $delete ) if ($index_authorities); > > my $slice_index = 0; > my $slice_count = ( $processes //= 1 ); > my %iterator_options; > >-if ($slice_count > 1) { >+if ( $slice_count > 1 ) { >+ > # Fire up child processes for processing slices from 2 on. This main process will handle slice 1. > $slice_index = 0; >- for (my $proc = 1; $proc < $slice_count; $proc++) { >+ for ( my $proc = 1 ; $proc < $slice_count ; $proc++ ) { > my $pid = fork(); > die "Failed to fork a child process\n" unless defined $pid; >- if ($pid == 0) { >+ if ( $pid == 0 ) { >+ > # Child process, give it a slice to process > $slice_index = $proc; > last; > } > } >+ > # Fudge the commit count a bit to spread out the Elasticsearch commits > $commit *= 1 + 0.10 * $slice_index; >- $commit = int( $commit ); >- _log(1, "Processing slice @{[$slice_index + 1]} of $slice_count\n"); >+ $commit = int($commit); >+ _log( 1, "Processing slice @{[$slice_index + 1]} of $slice_count\n" ); > $iterator_options{slice} = { index => $slice_index, count => $slice_count }; > } > >-if( $desc ){ >+if ($desc) { > $iterator_options{desc} = 1; > } > >+if ($where) { >+ $iterator_options{where} = $where; >+} >+ > my $next; > if ($index_biblios) { >- _log(1, "Indexing biblios\n"); >+ _log( 1, "Indexing biblios\n" ); > if (@biblionumbers) { > $next = sub { > my $r = shift @biblionumbers; > return () unless defined $r; >- return ($r, Koha::BiblioUtils->get_from_biblionumber($r, item_data => 1 )); >+ return ( $r, Koha::BiblioUtils->get_from_biblionumber( $r, item_data => 1 ) ); > }; > } else { > my $records = Koha::BiblioUtils->get_all_biblios_iterator(%iterator_options); >@@ -212,16 +225,16 @@ if ($index_biblios) { > $records->next(); > } > } >- _do_reindex($next, $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX); >+ _do_reindex( $next, $Koha::SearchEngine::Elasticsearch::BIBLIOS_INDEX ); > } > if ($index_authorities) { >- _log(1, "Indexing authorities\n"); >+ _log( 1, "Indexing authorities\n" ); > if (@authids) { > $next = sub { > my $r = shift @authids; > return () unless defined $r; > my $a = Koha::MetadataRecord::Authority->get_from_authid($r); >- return ($r, $a); >+ return ( $r, $a ); > }; > } else { > my $records = Koha::MetadataRecord::Authority->get_all_authorities_iterator(%iterator_options); >@@ -229,12 +242,13 @@ if ($index_authorities) { > $records->next(); > } > } >- _do_reindex($next, $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX); >+ _do_reindex( $next, $Koha::SearchEngine::Elasticsearch::AUTHORITIES_INDEX ); > } > >-if ($slice_index == 0) { >+if ( $slice_index == 0 ) { >+ > # Main process, wait for children >- for (my $proc = 1; $proc < $processes; $proc++) { >+ for ( my $proc = 1 ; $proc < $processes ; $proc++ ) { > wait(); > } > } >@@ -252,21 +266,22 @@ Checks the index state and recreates it if requested. > sub _verify_index_state { > my ( $index_name, $recreate ) = @_; > >- _log(1, "Checking state of $index_name index\n"); >+ _log( 1, "Checking state of $index_name index\n" ); > my $indexer = Koha::SearchEngine::Elasticsearch::Indexer->new( { index => $index_name } ); > > if ($recreate) { >- _log(1, "Dropping and recreating $index_name index\n"); >+ _log( 1, "Dropping and recreating $index_name index\n" ); > $indexer->drop_index() if $indexer->index_exists(); > $indexer->create_index(); >- } >- elsif (!$indexer->index_exists) { >+ } elsif ( !$indexer->index_exists ) { >+ > # Create index if does not exist > $indexer->create_index(); >- } elsif ($indexer->is_index_status_ok) { >+ } elsif ( $indexer->is_index_status_ok ) { >+ > # Update mapping unless index is some kind of problematic state > $indexer->update_mappings(); >- } elsif ($indexer->is_index_status_recreate_required) { >+ } elsif ( $indexer->is_index_status_recreate_required ) { > warn qq/Index "$index_name" has status "recreate required", suggesting it should be recreated/; > } > } >@@ -293,7 +308,7 @@ sub _do_reindex { > my $record = $record->record; > $count++; > if ( $verbose == 1 ) { >- _log( 1, "$count records processed\n" ) if ( $count % 1000 == 0); >+ _log( 1, "$count records processed\n" ) if ( $count % 1000 == 0 ); > } else { > _log( 2, "$id\n" ); > } >@@ -303,13 +318,13 @@ sub _do_reindex { > if ( !( --$commit_count ) ) { > _log( 1, "Committing $commit records...\n" ); > my $response; >- try{ >+ try { > $response = $indexer->update_index( \@id_buffer, \@commit_buffer ); > _handle_response($response); > _log( 1, "Commit complete\n" ); > } catch { >- _log(1,"Elasticsearch exception thrown: ".$_->type."\n"); >- _log(2,"Details: ".$_->details."\n"); >+ _log( 1, "Elasticsearch exception thrown: " . $_->type . "\n" ); >+ _log( 2, "Details: " . $_->details . "\n" ); > }; > $commit_count = $commit; > @id_buffer = (); >@@ -333,6 +348,7 @@ Checks some basic stuff to ensure that it's sane before we start. > =cut > > sub _sanity_check { >+ > # Do we have an elasticsearch block defined? > my $conf = C4::Context->config('elasticsearch'); > die "No 'elasticsearch' block is defined in koha-conf.xml.\n" if ( !$conf ); >@@ -346,14 +362,17 @@ Parse the return from update_index and display errors depending on verbosity of > > sub _handle_response { > my ($response) = @_; >- if( $response->{errors} eq 'true' ){ >+ if ( $response->{errors} eq 'true' ) { > _log( 1, "There were errors during indexing\n" ); >- if ( $verbose > 1 ){ >- foreach my $item (@{$response->{items}}){ >+ if ( $verbose > 1 ) { >+ foreach my $item ( @{ $response->{items} } ) { > next unless defined $item->{index}->{error}; >- print "Record #" . $item->{index}->{_id} . " " . >- $item->{index}->{error}->{reason} . " (" . $item->{index}->{error}->{type} . ") : " . >- $item->{index}->{error}->{caused_by}->{type} . " (" . $item->{index}->{error}->{caused_by}->{reason} . ")\n"; >+ print "Record #" >+ . $item->{index}->{_id} . " " >+ . $item->{index}->{error}->{reason} . " (" >+ . $item->{index}->{error}->{type} . ") : " >+ . $item->{index}->{error}->{caused_by}->{type} . " (" >+ . $item->{index}->{error}->{caused_by}->{reason} . ")\n"; > } > } > } >@@ -371,7 +390,7 @@ include a trailing newline automatically. > =cut > > sub _log { >- my ($level, $msg) = @_; >+ my ( $level, $msg ) = @_; > >- print "[$$] $msg" if ($verbose >= $level); >+ print "[$$] $msg" if ( $verbose >= $level ); > } >-- >2.30.2
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=162015">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 35345
:
159044
|
162015
|
164118
|
164157