[18.0] queue_job: refactor job acquisition #866
Conversation
|
Hi @guewen, |
sbidoul
force-pushed
the
18.0-refactor-job-acquisition-sbi
branch
2 times, most recently
from
265b06e to
8d0b9c9
Compare
sbidoul
changed the title
sbidoul
force-pushed
the
18.0-refactor-job-acquisition-sbi
branch
2 times, most recently
from
eff0043 to
d6c907a
Compare
sbidoul
force-pushed
the
18.0-refactor-job-acquisition-sbi
branch
from
d6c907a to
367ab80
Compare
| " to make this tests work", | ||
| ) | ||
|
|
||
| return job |
|
This is ready to review. It can be tested with a small number of workers and a root channel capacity greater than the number of workers and test jobs of duration >= 10 sec. If the test graph has enough parallelism, you will see warnings about dead jobs being requeued (the jobs in state enqueued that are not picked up by workers in due time), but each job should execute only once. Example: The |
|
I've also tested this locally and and it seems to work as intended |
sbidoul
force-pushed
the
18.0-refactor-job-acquisition-sbi
branch
from
367ab80 to
69acde4
Compare
In this commit we cleanly separate the job acquisition (i.e. verifying the job is in the exepected state, marking it started and locking it) from job execution. We also avoid trying to start the job if it is already locked by using SKIP LOCKED and exiting early. Indeed in such situations the job is likely already being handled by another worker so there is no point trying to start it, so we exit early and let it be handled either by the other worker or the dead job requeuer.
Extract the logic to run one job out of the /queue_job/runjob route. Towards making this logic reusable in other job executors.
sbidoul
force-pushed
the
18.0-refactor-job-acquisition-sbi
branch
from
69acde4 to
9937ed0
Compare
Towards making this logic reusable.
|
Thanks for the review and testing @hoangtrann ! I added a couple of commits to split out the job execution logic to class method so it is reusable from outside the controller and open the door to experiments such as #871. |
|
/ocabot merge patch |
|
On my way to merge this fine PR! |
|
Congratulations, your PR was merged at 48f5707. Thanks a lot for contributing to OCA. ❤️ |
4 participants
In this PR we cleanly separate the job acquisition (i.e. verifying the job is in the exepected state, marking it started and locking it) from job execution.
We also avoid trying to start the job if it is already locked by using SKIP LOCKED and exiting early. Indeed, in such situations, the job is likely already being handled by another worker so there is no point trying to start it, so we exit early and let it be handled either by the other worker or the dead job requeuer.
Following-up on #859 (comment)
maybe fixes #858