ashb commented on code in PR #25161:
URL: https://github.com/apache/airflow/pull/25161#discussion_r935682849
##########
airflow/executors/local_executor.py:
##########
@@ -78,14 +79,17 @@ def execute_work(self, key: TaskInstanceKey, command:
CommandType) -> None:
self.log.info("%s running %s", self.__class__.__name__, command)
setproctitle(f"airflow worker -- LocalExecutor: {command}")
- if settings.EXECUTE_TASKS_NEW_PYTHON_INTERPRETER:
- state = self._execute_work_in_subprocess(command)
- else:
- state = self._execute_work_in_fork(command)
-
- self.result_queue.put((key, state))
- # Remove the command since the worker is done executing the task
- setproctitle("airflow worker -- LocalExecutor")
+ with _airflow_parsing_context_manager(
+ AirflowParsingContextType.TASK_EXECUTION, dag_id=key[0],
task_id=key[1]
+ ):
Review Comment:
Nit(?): I would have expected this parsing context to be set in the "child"
process only, not the parent. Is there a reason it was done this way?
##########
airflow/utils/dag_parsing_context.py:
##########
@@ -0,0 +1,73 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+import os
+from contextlib import contextmanager
+from enum import Enum
+from typing import NamedTuple, Optional
+
+
+class AirflowParsingContextType(Enum):
+ """Type of context of parsing the DAG."""
+
+ DAG_PROCESSOR = "DAG_PROCESSOR"
+ TASK_EXECUTION = "TASK_EXECUTION"
Review Comment:
I'm somewhat wary of this -- why do we want a DAG to ever know what mode it
is in? Shouldn't the mode be one of
a) Give me all the dags in the file (in which case `context.dag_id` is
None); or
b) Give me a specific DAG only?
##########
airflow/task/task_runner/standard_task_runner.py:
##########
@@ -84,12 +85,16 @@ def _start_by_fork(self):
if job_id is not None:
proc_title += " {0.job_id}"
setproctitle(proc_title.format(args))
-
return_code = 0
try:
# parse dag file since `airflow tasks run --local` does not
parse dag file
dag = get_dag(args.subdir, args.dag_id)
- args.func(args, dag=dag)
+ with _airflow_parsing_context_manager(
Review Comment:
Shouldn't the `dag = get_dag()` call be inside the context too?
##########
airflow/executors/local_executor.py:
##########
@@ -78,14 +79,17 @@ def execute_work(self, key: TaskInstanceKey, command:
CommandType) -> None:
self.log.info("%s running %s", self.__class__.__name__, command)
setproctitle(f"airflow worker -- LocalExecutor: {command}")
- if settings.EXECUTE_TASKS_NEW_PYTHON_INTERPRETER:
- state = self._execute_work_in_subprocess(command)
- else:
- state = self._execute_work_in_fork(command)
-
- self.result_queue.put((key, state))
- # Remove the command since the worker is done executing the task
- setproctitle("airflow worker -- LocalExecutor")
+ with _airflow_parsing_context_manager(
+ AirflowParsingContextType.TASK_EXECUTION, dag_id=key[0],
task_id=key[1]
+ ):
Review Comment:
It looks like it _is_ done there too. In which case we shouldn't need it
here too?
##########
docs/apache-airflow/howto/dynamic-dag-generation.rst:
##########
@@ -140,3 +140,55 @@ Each of them can run separately with related configuration
.. warning::
Using this practice, pay attention to "late binding" behaviour in Python
loops. See `that GitHub discussion
<https://github.com/apache/airflow/discussions/21278#discussioncomment-2103559>`_
for more details
+
+
+Optimizing DAG parsing delays during execution
+----------------------------------------------
+
+Sometimes when you generate a lot of Dynamic DAGs from a single DAG file, it
might cause unnecessary delays
+when the DAG file is parsed during task execution. The impact is a delay
before a task starts.
+
+Why is this happening? You might not be aware but just before your task is
executed,
+Airflow parses the Python file the DAG comes from.
+
+The Airflow Scheduler (or DAG Processor) requires loading of a complete DAG
file to process all metadata.
+However, task execution requires only a single DAG object to execute a task.
Knowing this, we can
+skip the generation of unnecessary DAG objects when a task is executed,
shortening the parsing time.
+This optimization is most effective when the number of generated DAGs is high.
+
+There is an experimental approach that you can take to optimize this
behaviour. Note that it is not always
+possible to use (for example when generation of subsequent DAGs depends on the
previous DAGs) or when
+there are some side-effects of your DAGs generation. Also the code snippet
below is pretty complex and while
+we tested it and it works in most circumstances, there might be cases where
detection of the currently
+parsed DAG will fail and it will revert to creating all the DAGs or fail. Use
this solution with care and
+test it thoroughly.
+
+A nice example of performance improvements you can gain is shown in the
+`Airflow's Magic Loop
<https://medium.com/apache-airflow/airflows-magic-loop-ec424b05b629>`_ blog post
+that describes how parsing during task execution was reduced from 120 seconds
to 200 ms.
+
+In Airflow 2.4 the following variables you can use
+``airflow.utils.dag_parsing_context import get_parsing_context`` method to
retrieve the current context.
+
+Upon iterating over the collection of things to generate DAGs for, you can use
the context to determine
+whether you need to generate all DAG objects (when parsing in the DAG File
processor), or to generate only
+a single DAG object (when executing the task):
+
+.. code-block:: python
+ :emphasize-lines: 6,7,8,12,13
+
+ from airflow.models.dag import DAG
+ from airflow.utils.dag_parsing_context import get_parsing_context,
AirflowParsingContextType
+
+ current_dag = None
+ parsing_context = get_parsing_context()
+ if parsing_context and parsing_context.context_type ==
AirflowParsingContextType.TASK_EXECUTION:
+ current_dag = parsing_context.dag_id
Review Comment:
Further to my previous comment, I'd be happier if this was just:
```suggestion
current_dag = parsing_context.dag_id
```
--
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]
