potiuk commented on a change in pull request #21731:
URL: https://github.com/apache/airflow/pull/21731#discussion_r820693723
##########
File path: airflow/jobs/scheduler_job.py
##########
@@ -858,7 +865,7 @@ def _do_scheduling(self, session) -> int:
self.log.error("DAG '%s' not found in serialized_dag
table", dag_run.dag_id)
continue
- self._send_dag_callbacks_to_processor(dag, callback_to_run)
+ callbacks_to_send.append((dag, callback_to_run))
Review comment:
To be honest I think that guard couid be stopped here and restart later
(or maybe even it is not needed later @ashb? I tihink the main reason of the
"prohibit_commit" guard was to make sure the DagRun rows are locked, but after
the commit above - they are already freed anyway. I think the guard is used
from this place on to make sure that simply none of the code is using
accidental commit (and it does not protect the DagRun rows which are freed at
this point), so conceptually, I see no problem if the Callbacks are run outside
of the guard and we have a separate guard for critical section.
That would make quite a lot of sense actually.
--
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]
