From 02e4e3f8f685be83b3cca7d4b13bb6ad5fcdf302 Mon Sep 17 00:00:00 2001 From: Marcel de Rooy Date: Mon, 5 Sep 2022 11:57:52 +0000 Subject: [PATCH] Bug 27421: (QA follow-up) BatchCommitImportRecords needs param for skipping commits Content-Type: text/plain; charset=utf-8 When you submit a background jobs, and it fails, you do not expect partial results in the database. Note that when the Background feature would support a partially completed status, things might change again. Note that the >0 test was superfluous if you check for ^\d+$. Test plan: Run t/db_dependent/Koha/BackgroundJobs/StageMARCForImport.t Note: This serves to verify that it still runs as expected. The test plan of the following patch covers the new param. Signed-off-by: Marcel de Rooy --- C4/ImportBatch.pm | 38 +++++++++------------ Koha/BackgroundJob/MARCImportCommitBatch.pm | 13 +++---- 2 files changed, 24 insertions(+), 27 deletions(-) diff --git a/C4/ImportBatch.pm b/C4/ImportBatch.pm index bd51a5c738..96ee1c45a7 100644 --- a/C4/ImportBatch.pm +++ b/C4/ImportBatch.pm @@ -509,28 +509,23 @@ sub BatchFindDuplicates { =head2 BatchCommitRecords - my ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored) = - BatchCommitRecords($batch_id, $framework, - $progress_interval, $progress_callback); + my ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored) = + BatchCommitRecords( $batch_id, $framework, $progress_interval, $progress_callback, $params ); + + Parameter skip_intermediate_commit does what is says. =cut sub BatchCommitRecords { - my $batch_id = shift; - my $framework = shift; + my ( $batch_id, $framework, $progress_interval, $progress_callback, $params ) = @_; + my $skip_intermediate_commit = $params->{skip_intermediate_commit}; + $progress_interval = 0 unless $progress_interval && $progress_interval =~ /^\d+$/; + $progress_interval = 0 unless ref($progress_callback) eq 'CODE'; my $schema = Koha::Database->schema; - - # optional callback to monitor status - # of job - my $progress_interval = 0; - my $progress_callback = undef; - if ($#_ == 1) { - $progress_interval = shift; - $progress_callback = shift; - $progress_interval = 0 unless $progress_interval =~ /^\d+$/ and $progress_interval > 0; - $progress_interval = 0 unless 'CODE' eq ref $progress_callback; - } + $schema->txn_begin; + # NOTE: Moved this transaction to the front of the routine. Note that inside the while loop below + # transactions may be committed and started too again. The final commit is close to the end. my $record_type; my $num_added = 0; @@ -560,7 +555,6 @@ sub BatchCommitRecords { my $rec_num = 0; my @biblio_ids; - $schema->txn_begin; # We commit in a transaction while (my $rowref = $sth->fetchrow_hashref) { $record_type = $rowref->{'record_type'}; @@ -568,9 +562,9 @@ sub BatchCommitRecords { if ($progress_interval and (0 == ($rec_num % $progress_interval))) { # report progress and commit - $schema->txn_commit; + $schema->txn_commit unless $skip_intermediate_commit; &$progress_callback( $rec_num ); - $schema->txn_begin; + $schema->txn_begin unless $skip_intermediate_commit; } if ($rowref->{'status'} eq 'error' or $rowref->{'status'} eq 'imported') { $num_ignored++; @@ -700,8 +694,6 @@ sub BatchCommitRecords { &$progress_callback($rec_num); } - $schema->txn_commit; # Commit final records that may not have hit callback threshold - $sth->finish(); if ( @biblio_ids ) { @@ -710,6 +702,10 @@ sub BatchCommitRecords { } SetImportBatchStatus($batch_id, 'imported'); + + # Moved final commit to the end + $schema->txn_commit; + return ($num_added, $num_updated, $num_items_added, $num_items_replaced, $num_items_errored, $num_ignored); } diff --git a/Koha/BackgroundJob/MARCImportCommitBatch.pm b/Koha/BackgroundJob/MARCImportCommitBatch.pm index 57faa99c51..23a8123756 100644 --- a/Koha/BackgroundJob/MARCImportCommitBatch.pm +++ b/Koha/BackgroundJob/MARCImportCommitBatch.pm @@ -67,12 +67,13 @@ sub process { try { my $size = Koha::Import::Records->search({ import_batch_id => $import_batch_id })->count; $self->size($size)->store; - ( - $num_added, $num_updated, $num_items_added, - $num_items_replaced, $num_items_errored, $num_ignored - ) - = BatchCommitRecords( $import_batch_id, $frameworkcode, 50, - sub { my $job_progress = shift; $self->progress( $job_progress )->store } ); + ( $num_added, $num_updated, $num_items_added, + $num_items_replaced, $num_items_errored, $num_ignored ) = + BatchCommitRecords( + $import_batch_id, $frameworkcode, 50, + sub { my $job_progress = shift; $self->progress( $job_progress )->store }, + { skip_intermediate_commit => 1 }, + ); } catch { warn $_; -- 2.20.1