From 40a329474486ffdef145466ddb66d65060d656ac Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 24 Feb 2022 10:41:16 +0100 Subject: [PATCH] Bug 30172: Prevent race condition when enqueuing a new task As we are sending the job to the rabbitmq before in the transaction, the worker can receive the job to process before the transaction committed. Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize Signed-off-by: Tomas Cohen Arazi --- Koha/BackgroundJob.pm | 67 +++++++++---------- .../prog/en/modules/admin/background_jobs.tt | 2 + 2 files changed, 33 insertions(+), 36 deletions(-) diff --git a/Koha/BackgroundJob.pm b/Koha/BackgroundJob.pm index cb8a71fb04..5fa977977f 100644 --- a/Koha/BackgroundJob.pm +++ b/Koha/BackgroundJob.pm @@ -100,44 +100,39 @@ sub enqueue { my $borrowernumber = C4::Context->userenv->{number}; # FIXME Handle non GUI calls my $json_args = encode_json $job_args; - my $job_id; - $self->_result->result_source->schema->txn_do( - sub { - $self->set( - { - status => 'new', - type => $job_type, - size => $job_size, - data => $json_args, - enqueued_on => dt_from_string, - borrowernumber => $borrowernumber, - } - )->store; - - $job_id = $self->id; - $job_args->{job_id} = $job_id; - $json_args = encode_json $job_args; - - try { - my $conn = $self->connect; - # This namespace is wrong, it must be a vhost instead. - # But to do so it needs to be created on the server => much more work when a new Koha instance is created. - # Also, here we just want the Koha instance's name, but it's not in the config... - # Picking a random id (memcached_namespace) from the config - my $namespace = C4::Context->config('memcached_namespace'); - $conn->send_with_receipt( { destination => sprintf("/queue/%s-%s", $namespace, $job_type), body => $json_args } ) - or Koha::Exceptions::Exception->throw('Job has not been enqueued'); - } catch { - if ( ref($_) eq 'Koha::Exceptions::Exception' ) { - $_->rethrow; - } else { - warn sprintf "The job has not been sent to the message broker: (%s)", $_; - } - }; + $self->set( + { + status => 'new', + type => $job_type, + size => $job_size, + data => $json_args, + enqueued_on => dt_from_string, + borrowernumber => $borrowernumber, } - ); + )->store; + + $job_args->{job_id} = $self->id; + $json_args = encode_json $job_args; + + try { + my $conn = $self->connect; + # This namespace is wrong, it must be a vhost instead. + # But to do so it needs to be created on the server => much more work when a new Koha instance is created. + # Also, here we just want the Koha instance's name, but it's not in the config... + # Picking a random id (memcached_namespace) from the config + my $namespace = C4::Context->config('memcached_namespace'); + $conn->send_with_receipt( { destination => sprintf("/queue/%s-%s", $namespace, $job_type), body => $json_args } ) + or Koha::Exceptions::Exception->throw('Job has not been enqueued'); + } catch { + $self->status('failed')->store; + if ( ref($_) eq 'Koha::Exceptions::Exception' ) { + $_->rethrow; + } else { + warn sprintf "The job has not been sent to the message broker: (%s)", $_; + } + }; - return $job_id; + return $self->id; } =head3 process diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt index f73e9222ae..861325d7ac 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/background_jobs.tt @@ -15,6 +15,8 @@ Started [% CASE "running" %] Running + [% CASE "failed" %] + Failed [% CASE # Default case %] [% job.status | html %] [% END -%] -- 2.39.5