Skip to content

[19.0][IMP] edi_queue_oca: cancel wait_dependencies jobs on exchange failure - #340

Open
AaronHForgeFlow wants to merge 1 commit into
OCA:19.0from
ForgeFlow:19.0-fix-edi_queue_oca-cancel-zombie-jobs
Open

[19.0][IMP] edi_queue_oca: cancel wait_dependencies jobs on exchange failure#340
AaronHForgeFlow wants to merge 1 commit into
OCA:19.0from
ForgeFlow:19.0-fix-edi_queue_oca-cancel-zombie-jobs

Conversation

@AaronHForgeFlow

Copy link
Copy Markdown

When a chained queue job (such as generate->send or receive->process) fails, downstream dependent jobs remain permanently stuck in wait_dependencies because queue_job only cancels dependent jobs upon explicit job cancellation.

Ensure _job_on_fail_update cancels any wait_dependencies queue jobs belonging to the errored exchange record so zombie jobs do not linger.

@simahawk

simahawk commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

If I get it right, when a send job fails you will cancel it... I don't think this is ideal at all.
If you cancel it, you'll miss it and you won't be able to re-schedule it.
Am I missing anything?

@AaronHForgeFlow

Copy link
Copy Markdown
Author

This targets only downstream child jobs in state wait_dependencies (for example, the chained send job when generate has failed) When generate fails, the child send job is stuck in wait_dependencies waiting for its parent generate to reach done

should I mark the job as failed instead?

@simahawk

simahawk commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

I see this useful only in the situation where a second pair of jobs does the work and leaves the old ones stale:

  • job11: generate -> fails
  • job12: send -> stale
  • job21: generate -> done
  • job22: send -> done

If I get it right, with your change as soon as a generate fails, the send will be discarded leaving only the generate job in failed state. If you run only this job, no send will happen unless you also rely on crons but that can cause additional delays. Correct?

should I mark the job as failed instead?

only if requeueing the generate job causes the requeueing of the chained job too (which I doubt).

@AaronHForgeFlow

Copy link
Copy Markdown
Author

If you run only this job, no send will happen unless you also rely on crons but that can cause additional delays. Correct?

Yes, correct. In my case I usually rely on the user action, they will create a new exchange record leaving the previous one as canceled or failed. I am open to suggestions, I am not sure if this is the best approach.

@simahawk

simahawk commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

TBH I don't have a precise answer. Options:

  1. when a full process is over input/output can spawn a GC job to cancel stale jobs for that record
  2. add GC to the existing crons to cancel stale jobs
  3. any other option?

I tend to say that opt 2 is the cleanest and safest.

@AaronHForgeFlow
AaronHForgeFlow force-pushed the 19.0-fix-edi_queue_oca-cancel-zombie-jobs branch 3 times, most recently from 8c77e42 to 308707d Compare September 8, 2026 14:16
@AaronHForgeFlow

Copy link
Copy Markdown
Author

Added a system parameter so the zombie jobs can be collected some time. If this is too aggresive maybe we can add a configuration or a new field in the backend so it does not affect current instances.

@AaronHForgeFlow
AaronHForgeFlow force-pushed the 19.0-fix-edi_queue_oca-cancel-zombie-jobs branch from 308707d to 1604a2d Compare September 8, 2026 14:22
@AaronHForgeFlow AaronHForgeFlow changed the title [19.0][FIX] edi_queue_oca: cancel wait_dependencies jobs on exchange failure [19.0][IMP] edi_queue_oca: cancel wait_dependencies jobs on exchange failure Sep 8, 2026
self._gc_stale_exchange_jobs()
return res

def _gc_stale_exchange_jobs_grace_hours(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can you pls prefix all method names w/ _job_*?


def _cron_check_input_exchange_sync(self, **kw):
res = super()._cron_check_input_exchange_sync(**kw)
self._gc_stale_exchange_jobs()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This means that the GC will run twice and we don't have any way to turn it off if needed.
Options:

  1. move it to its own cron that can be tuned/disabled if needed (disabled by default)
  2. if the config param is zero the whole feature is disable (default)

return bool(blocking) and not any(x in LIVE_JOB_STATES for x in blocking)

def _gc_stale_exchange_jobs(self):
"""Cancel exchange jobs that can never be woken up.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like to respect this PEP for multi-line docstrings https://peps.python.org/pep-0257/#multi-line-docstrings

)
if stale:
_logger.info("EDI exchange GC: cancelling %d stale jobs.", len(stale))
stale.button_cancelled()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

shall we post a message or override the result message to make clear that it was canceled by GC?

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants