airflow-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [airflow] ashb commented on a change in pull request #6792: [AIRFLOW-5930] Use cached-SQL query building for hot-path queries
Date Thu, 12 Dec 2019 21:04:55 GMT
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:
users@infra.apache.org


With regards,
Apache Git Services

Mime
View raw message