Don't queue runs on DatasetEvent for disabled DAGs#38891
Conversation
boring-cyborg
Bot
added
the
area:Scheduler
|
I decided to not consider catchup at all here, and make the behavior change as simple as possible. A news fragment has been added to note the breaking change. |
This adds an additional filter on consuming DAG, and only create a DatasetDagRunQueue entry if the DAG is both active (i.e. still exists in a DAG file) and unpaused. There are still questions unanswered: 1. Do we consider catchup? The immediate problem here is catchup is not stored on DagModel, and checking it from DAG creates significant overhead. 2. Do we just change the behavior outright? This is a breaking change since users relying on a disabled DAG can still accumulating events (that trigger runs later) as a feature will see it broken. We can maybe categorize the current behavior as a bug.
uranusjr
force-pushed
the
do-not-queue-inactive-dags
branch
from
df7d731 to
c74b69d
Compare
What does it mean in cases where dag has both dataset and time base scheduling? |
|
The catchup flag still affects time-based scheduling (and only that). I’ll call that out explicitly in the fragment. |
|
@uranusjr / @eladkal but isn't above implementation will miss the use case in which any paused dag will just lose dataset events that came when DAG was paused / disabled. I agree on considering To me default implementation should be just delivering all the events dag received during the time it was paused / disable without considering catchup. For the usecase when dag is deleted and comeback with same dag_id loosing dataset event is ok. |
|
The question, if you don’t want to miss the events, why do you also want to pause the DAG? If pausing a DAG doesn’t make it miss events, all those will still run when the DAG is unpaused. Why is it so important to pause the DAG and delay the runs? |
|
@uranusjr i checked the implementation and it is already perfectly aligned with what i said earlier. As all the dataset events will be received by the paused/deleted/disabled dag once its enabled and a new dataset event is received as then it was have an entry in the DatasetDagRunQueue and that perfectly make sense. When i said miss the events i meant dataset events and not the DatasetDagRunQueue events. Apologies for confusion earlier |
5 participants
Maybe fix #38826?
This adds an additional filter on consuming DAG, and only create a DatasetDagRunQueue entry if the DAG is both active (i.e. still exists in a DAG file) and unpaused.
There are still questions unanswered: