Skip to content

Reset job disabling timer on adding the job again - #30358

Merged
juliusknorr merged 1 commit into
masterfrom
bugfix/noid/job-disabled
Jan 4, 2022
Merged

Reset job disabling timer on adding the job again#30358
juliusknorr merged 1 commit into
masterfrom
bugfix/noid/job-disabled

Conversation

@juliusknorr

@juliusknorrjuliusknorr commented Dec 21, 2021

Copy link
Copy Markdown
Member

If a job run is triggered while having the app disabled the next execution will be delayed for 12 hours

// set the last_checked to 12h in the future to not check failing jobs all over again
$reset = $this->connection->getQueryBuilder();
$reset->update('jobs')
->set('reserved_at', $reset->expr()->literal(0, IQueryBuilder::PARAM_INT))
->set('last_checked', $reset->createNamedParameter($this->timeFactory->getTime() + 12 * 3600, IQueryBuilder::PARAM_INT))
->where($reset->expr()->eq('id', $reset->createNamedParameter($row['id'], IQueryBuilder::PARAM_INT)));
$reset->execute();

This PR ensures that the job is rescheduled immediately if an app gets enabled and the jobs from appinfo.xml get parsed again in

\OC_App::setupBackgroundJobs($info['background-jobs']);

Fixes#30285

@juliusknorrjuliusknorr added bug 3. to review Waiting for reviews labels Dec 21, 2021
@juliusknorrjuliusknorr added this to the Nextcloud 24 milestone Dec 21, 2021
@juliusknorr
juliusknorr requested review from a team, ArtificialOwl, CarlSchwan, PVince81 and artonge and removed request for a teamDecember 21, 2021 07:09
Comment threadlib/private/BackgroundJob/JobList.php Outdated
@juliusknorr
juliusknorrforce-pushed the bugfix/noid/job-disabled branch from 60c3f15 to 28a7373CompareDecember 22, 2021 10:45
@artonge

Copy link
Copy Markdown
Collaborator

Can we prevent duplicate code? But maybe I missed some tiny difference :).

publicfunctionadd($job, $argument = null) {
if ($jobinstanceof IJob) {
$class = get_class($job);
} else {
$class = $job;
}
$argument = json_encode($argument);
if (strlen($argument) > 4000) {
thrownew \InvalidArgumentException('Background job arguments can\'t exceed 4000 characters (json encoded)');
}
$query = $this->connection->getQueryBuilder();
if (!$this->has($job, $argument)) {
$query->insert('jobs')
->values([
'class' => $query->createNamedParameter($class),
'argument' => $query->createNamedParameter($argument),
'last_run' => $query->createNamedParameter(0, IQueryBuilder::PARAM_INT),
'last_checked' => $query->createNamedParameter($this->timeFactory->getTime(), IQueryBuilder::PARAM_INT),
]);
} else {
$query->update('jobs')
->set('reserved_at', $query->expr()->literal(0, IQueryBuilder::PARAM_INT))
->set('last_checked', $query->createNamedParameter($this->timeFactory->getTime(), IQueryBuilder::PARAM_INT))
->where($query->expr()->eq('class', $query->createNamedParameter($class)))
->andWhere($query->expr()->eq('argument', $query->createNamedParameter($argument)));
}
$query->execute();
}

@juliusknorr

Copy link
Copy Markdown
MemberAuthor

@artonge Pushed a fixup with some minor adjustment because $this->has($job, $argument) requires the original argument instead of the json_encoded one.

@artongeartonge left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can move $query->execute(); out of the if, but ok for me :)

@juliusknorrjuliusknorr mentioned this pull request Jan 3, 2022
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
juliusknorrforce-pushed the bugfix/noid/job-disabled branch from 221d0e6 to b30193eCompareJanuary 3, 2022 11:12
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

We can move $query->execute(); out of the if, but ok for me :)

Rebased, squashed, replaced deprecated execute with executeStatement and moved it out of the if as well ;)

@juliusknorr
juliusknorr merged commit 16fc0da into masterJan 4, 2022
@juliusknorr
juliusknorr deleted the bugfix/noid/job-disabled branch January 4, 2022 08:21
@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable23

@juliusknorr

Copy link
Copy Markdown
MemberAuthor

/backport to stable22

@MichaIng

Copy link
Copy Markdown
Member

/backport to stable21

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewsbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background job execution delayed by 12 hours during app reenabling

4 participants

@juliusknorr@artonge@MichaIng@skjnldsv