We've circled this question a few times and I am trying to nail down an answer that we can implement.
Given this situation: You create a Dag with a Deadline Alert for one hour from the Dag's queued_at time. Ten minutes in, the Dag fails. What do you expect to happen? There are three clear options here: 1. The Deadline callback fires at ten minutes (on Dagrun failure). If the Deadline Alert means "I expect this to succeed in under an hour," then after 10 minutes we know it can't succeed, so fire now. This is what PR #68961 implements (behind an opt-in flag). 2. The callback never fires. If the Deadline Alert means "I expect this to finish within an hour," then a failed run has finished, so the deadline is satisfied and the pending alert is pruned, the same way we already prune it on success. 3. The callback fires at the one-hour mark regardless. The deadline timer runs to expiry independent of the run's state, so a failed run still triggers the alert 50 minutes after it died. This is the current behaviour on main. What the docs say right now (emphasis mine, and keeping in mind we can change this based on the decision here): `core-concepts/dags.rst` says: "This example will send an email notification if the Dag **hasn't finished** 30 minutes after it was queued." `howto/deadline-alerts.rst` says the DAGRUN_LOGICAL_DATE reference "would trigger the alert if the Dag **hasn't completed** 15 minutes after it was scheduled to start," and describes AVERAGE_RUNTIME as alerting when a "Dag is **running longer** than expected." So the docs currently frame it as (not finished | not completed | still running), they do not say `not succeeded`. By that phrasing, a failed run HAS finished, which points at option 2, and the user can use the existing `on_failure_callback` if that was what they wanted . But `howto/sla-to-deadlines.rst` describes the mechanism differently: "When a Dag run starts, calculate and store (DeadlineReference + interval). The scheduler loop then checks periodically ... **if any of those times have passed** then execute callback," and stresses the callback "will execute 'immediately' ... and not wait until the Dag finishes first." That description has no step that cancels a pending deadline when the run ends, which is the only place option 3 (today's behaviour) finds any support. So the documentation contradicts itself: the conceptual pages point to option 1 or option 2, depending on how you read it, and the migration guide's mechanism describes option 3. So I am asking: which of the three SHOULD be the behaviour, and should any of the others be an opt-in alternative? My own opinion is that option 1 was the intent, and I'd like to see it implemented as a bug fix rather than an optional flag: a deadline means "did not succeed in time," a failed run cannot succeed, so fire. If we keep a flag at all I'd make option 1 the default and allow an opt-out to prune on failure (option 2) for teams who already get failure signal from on_failure_callback and don't want a duplicate. The counter-argument worth weighing, raised on #68961: an on-call who clears a failed run and retries would, under option 1, lose the "still hasn't recovered by the deadline" signal, which is an argument for option 2 or for keeping timing-based firing available. Could you reply with the number you'd expect as a user, and whether you'd want the other behaviours available as an option? If there's rough consensus I'll make the docs and #68961 match it. Thanks, - ferruzzi --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
