ashb commented on a change in pull request #6792: [AIRFLOW-5930] Use cached-SQL
query building for hot-path queries
URL: https://github.com/apache/airflow/pull/6792#discussion_r357370216
##########
File path: airflow/models/dagrun.py
##########
@@ -286,25 +320,27 @@ def update_state(self, session=None):
session=session
)
none_depends_on_past = all(not t.task.depends_on_past for t in
unfinished_tasks)
- none_task_concurrency = all(t.task.task_concurrency is None
- for t in unfinished_tasks)
- # small speed up
- if unfinished_tasks and none_depends_on_past and none_task_concurrency:
- # todo: this can actually get pretty slow: one task costs between
0.01-015s
- no_dependencies_met = True
- for ut in unfinished_tasks:
- # We need to flag upstream and check for changes because
upstream
- # failures/re-schedules can result in deadlock false positives
- old_state = ut.state
- deps_met = ut.are_dependencies_met(
- dep_context=DepContext(
- flag_upstream_failed=True,
- ignore_in_retry_period=True,
- ignore_in_reschedule_period=True),
- session=session)
- if deps_met or old_state != ut.current_state(session=session):
- no_dependencies_met = False
- break
+ none_task_concurrency = all(t.task.task_concurrency is None for t in
unfinished_tasks)
+
+ no_dependencies_met = True
+
+ dep_context = DepContext(flag_upstream_failed=True)
+
+ for ut in unfinished_tasks:
+ # We need to flag upstream and check for changes because upstream
+ # failures/re-schedules can result in deadlock false positives
+ old_state = ut.state
+ unmet_deps =
list(ut.get_failed_dep_statuses(dep_context=dep_context, session=session))
+ unmet_non_delay_dep = any(
+ unmet_dep for unmet_dep in unmet_deps
+ if unmet_dep.dep_name not in {NotInRetryPeriodDep.NAME,
ReadyToRescheduleDep.NAME}
+ )
+
+ state = ut.current_state(session=session)
Review comment:
This function showed up as a noticable chunk of the profling time for the
DagFileProcessor, so we should decide if it's needed.
Right now as it is (and was) called it re-used the same session, so would be
run in the same transaction. I think this makes the call pointless as it's
asking for the state of the TI, but since its in the same transaction it will
have a consistent view of the row, right?
I'm basing this off https://www.postgresql.org/docs/9.5/transaction-iso.html
which says
> Read Committed is the default isolation level in PostgreSQL. When a
transaction uses this isolation level, a SELECT query (without a FOR
UPDATE/SHARE clause) sees only data committed before the query began; it never
sees either uncommitted data or changes committed during query execution by
concurrent transactions. In effect, a SELECT query sees a snapshot of the
database as of the instant the query begins to run
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
