ephraimbuddy commented on pull request #21901: URL: https://github.com/apache/airflow/pull/21901#issuecomment-1061966077
> Just one worry. Are you sure there will be no situation when max_active_runs_reached will not lead to a stale Dags ? This is - effectively - caching of whether the max dag runs have been reached. The value is re-calculated regularly and updated when it changes, but are we absoluttely sure there is no way this re-calculation is always happening for all the dags (for example I can imagine situation where we reach max active dag_runs and (for whatever reason) we never enter the _max_active_runs_reached function to recalculate it. Yeah. That makes a lot of sense and I need to do some more testing. The > Thanks for the context! Perfect explanation. Now I understand it fully. > > Just one worry. Are you sure there will be no situation when `max_active_runs_reached` will not lead to a stale Dags ? This is - effectively - caching of whether the max dag runs have been reached. The value is re-calculated regularly and updated when it changes, but are we absoluttely sure there is no way this re-calculation is always happening for all the dags (for example I can imagine situation where we reach max active dag_runs and (for whatever reason) we never enter the `_max_active_runs_reached` function to recalculate it. > > I am not saying it is the case (I could not see any path that leads to it) but it's worth some careful review of the logic. > > Small NIT for the name of the method. I will do more testing on this but I expect it not to have stale dags because I'm still doing the same nullification but not doing a calculation of the next_dagrun after update. Thanks for pointing this out! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
