Skip to content

[FIX] base_bg: run one job per cron run and isolate the user notification - #494

Closed
rov-adhoc wants to merge 1 commit into
ingadhoc:18.0from
adhoc-dev:18.0-t-79313-rov
Closed

rov-adhoc wants to merge 1 commit into
ingadhoc:18.0from
adhoc-dev:18.0-t-79313-rov

Conversation

@rov-adhoc

Copy link
Copy Markdown
Contributor

Tarea: https://www.adhoc.inc/odoo/project.task/79313

Problem

limit_time_real_cron counts the whole cron run. In 18.0 _cron_run_enqueued_jobs(limit=5) chains up to 5 jobs in one run, so with jobs of about 10 minutes each (large bank statement imports) the worker passes the limit and is killed with SIGKILL. The jobs start again from zero.

Also, finish() and _notify_user() share the same commit. When the notification fails with could not serialize access due to concurrent update on UPDATE discuss_channel_member, the rollback discards the work of a job that already finished, and the retry runs it again.

Changes

  • _cron_run_enqueued_jobs: one job per run by default (limit is kept as a parameter). When enqueued jobs remain, the runner triggers itself again. This also covers a failed job that is enqueued again, which before waited for the 10 minute interval.
  • run(): commits the job work and finish() before notifying. The new _notify_result posts the notification in a savepoint and only logs an error, so the job stays done and is not run again.

Forward-port to 19.0

19.0 already runs one job per cron run, so the runner part and test_cron_runs_one_job_per_run_and_retriggers do not apply there and must be dropped. The notification fix does apply, adapted to the 19.0 run() (it handles errors without raising and calls invalidate_all()). That adaptation is ready and its test passes on 19.0 (59/59 base_bg tests on a fresh database).

Tests

  • test_run_notification_failure_keeps_job_done
  • test_cron_runs_one_job_per_run_and_retriggers

These 18.0 tests were not run locally (19.0 environment): runbot validates them.

…tion

The cron real-time limit (limit_time_real_cron) applies to the whole cron
run. The runner chained up to 5 jobs per run, so a few long jobs (e.g.
bank statement imports of about 10 minutes each) were enough for the
worker to be killed with SIGKILL, and the jobs were started again.

- _cron_run_enqueued_jobs processes one job per run by default and
  triggers the runner again while enqueued jobs remain.
- run() commits the job work and its finish() before notifying the user.
  The notification runs in a savepoint and an error there (e.g. a
  serialization failure on discuss_channel_member) is only logged, so a
  finished job is not rolled back nor run again.
@roboadhoc

Copy link
Copy Markdown
Contributor

Pull request status dashboard

@maq-adhoc

Copy link
Copy Markdown
Contributor

@roboadhoc r+ nobump

roboadhoc pushed a commit that referenced this pull request Nov 13, 2026
…tion

The cron real-time limit (limit_time_real_cron) applies to the whole cron
run. The runner chained up to 5 jobs per run, so a few long jobs (e.g.
bank statement imports of about 10 minutes each) were enough for the
worker to be killed with SIGKILL, and the jobs were started again.

- _cron_run_enqueued_jobs processes one job per run by default and
  triggers the runner again while enqueued jobs remain.
- run() commits the job work and its finish() before notifying the user.
  The notification runs in a savepoint and an error there (e.g. a
  serialization failure on discuss_channel_member) is only logged, so a
  finished job is not rolled back nor run again.

closes #494

Signed-off-by: Filoquin adhoc <[email protected]>
@roboadhoc roboadhoc closed this Nov 13, 2026
@roboadhoc
roboadhoc deleted the 18.0-t-79313-rov branch November 13, 2026 12:56

This branch was previously deployed

1 inactive deployment
merge — 2090330d Deployed Nov 13, 2026 by roboadhoc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants