[19.0][IMP] queue_job: prevent commit during queue job execution

[sbidoul]

Commiting in a job would release the job lock, causing spurious restarts by the dead jobs requeuer.

Very much draft and untested at this point.

See also discussion in #889

[OCA-git-bot]

Hi @guewen,
some modules you are maintaining are being modified, check this out!

[guewen]

As I understand, it is very likely to happen when using queue_job_cron, as a lot of cron methods do intermediate commits

[sbidoul]

Precisely, yes.

[sbidoul]

@guewen an alternative would be to acquire the job lock with a separate cursor, the drawback being doubling the number of postgres connections required for jobs.

I'm not sure how prevalent the problem is, so to understand better I'm inclined to merge this so affected users get a clear error instead of hard to diagnose issues with job running multiple times. And if it appears that commit in jobs is a really really important use case then we can consider alternatives.

[guewen]

I'm inclined to merge this so affected users get a clear error instead of hard to diagnose issues with job running multiple times.

Yes, even if crons/jobs are supposed to be idempotent, in case they are not, it can be devastating in some cases if they are run several times concurrently. A clear error is indeed much better than unexpected and erratic behavior.

And it is not blocking anybody because if a cron job is flagged as "run as job" and raises "Commit is forbidden in queue jobs" because of a commit, they can still run it as a normal cron. Thinking about it, we may add some details in the error message, ex "Commit is forbidden in queue jobs. If the current job is a cron running as job, it should be run as a normal job"?

[sbidoul]

I initially was trying assert env is job.env but that fails. I don't quite understand why, but that's unrelated to this PR. What is important is that job.perform() uses the same transaction as the one that is commited with env.cr.commit() 5 lines below.

[sbidoul]

This is now ready. I implemented your error message suggestion and added a way to test this interactively.

[guewen]

Thanks!

[RyanTranAVB]

Code LGTM

[sbidoul]

/ocabot merge minor

[OCA-git-bot]

This PR looks fantastic, let's merge it!
Prepared branch 19.0-ocabot-merge-pr-880-by-sbidoul-bump-minor, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 749b33f. Thanks a lot for contributing to OCA. ❤️