Skip to content

Commit

Permalink
Require GruTask in database before running a minion job
Browse files Browse the repository at this point in the history
It can happen that a GruTasks entry is added in a transaction and the minion
job already enqueued and started before the transaction is committed.

In that case we shouldn't start the minion job (the transaction could have
been rollbacked even).

The task needs to be deleted by the minion job in order to go on with
processing a job, and it can't delete it if it doesn't see the entry in the
database.

But requiring the entry before the start of the job is even more correct IMHO.

Issue: https://progress.opensuse.org/issues/165866
  • Loading branch information
perlpunk committed Sep 3, 2024
1 parent a013d38 commit e40ccc0
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 0 deletions.
6 changes: 6 additions & 0 deletions lib/OpenQA/Shared/GruJob.pm
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@ sub execute {
my $self = shift;

my $gru_id = $self->info->{notes}{gru_id};
if ($gru_id and not $self->info->{finished}) {
# We have a gru_id and this is the first run of the job
my $gru = $self->minion->app->schema->resultset('GruTasks')->find($gru_id);
# GruTask might not yet have landed in database due to open transaction
return $self->retry({delay => 2}) unless $gru;
}
my $err = $self->SUPER::execute;

# Non-Gru tasks
Expand Down
13 changes: 13 additions & 0 deletions t/14-grutasks.t
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ $t->app->minion->add_task(
return $job->retry({delay => 30})
unless my $guard = $job->minion->guard('limit_gru_retry_task', ONE_HOUR);
});
$t->app->minion->add_task(wait_for_grutask => sub (@) { undef });

# Gru task that reached failed/finished manually
# uncoverable statement
Expand Down Expand Up @@ -574,6 +575,18 @@ subtest 'Gru tasks retry' => sub {
is $t->app->minion->job($ids->{minion_id})->info->{state}, 'finished', 'minion job is finished';
};

subtest 'waiting gru job' => sub {
my $enq = $app->gru->enqueue(wait_for_grutask => {});
# Simulate that the gru task is not yet visible for the minion server
$schema->resultset('GruTasks')->find($enq->{gru_id})->delete;
my $worker = $app->minion->worker->register;
my $job = $worker->dequeue(0, {id => $enq->{minion_id}});
$job->execute;
is $job->info->{retries}, 1, 'job retried because there is no GruTasks entry';
is $job->info->{state}, 'inactive', 'state inactive because there is no GruTasks entry';
is $job->info->{finished}, undef, 'finished is not defined';
};

# prevent writing to a log file to enable use of combined_like in the following tests
$t->app->log(Mojo::Log->new(level => 'debug'));

Expand Down

0 comments on commit e40ccc0

Please sign in to comment.