potiuk commented on a change in pull request #20147:
URL: https://github.com/apache/airflow/pull/20147#discussion_r831896570



##########
File path: airflow/task/task_runner/base_task_runner.py
##########
@@ -57,7 +57,8 @@ def __init__(self, local_task_job):
             except AirflowConfigException:
                 self.run_as_user = None
 
-        self._error_file = NamedTemporaryFile(delete=True)
+        self._error_file = NamedTemporaryFile(delete=False)

Review comment:
       What I am saying is - that your test is wrong for default setting of 
Airlfow. If you do not understand how fork() works then you need to find out to 
be able to assess how the file handle closing will impact it:
   
   This is the part that is relevant.
   
   ```
       def _start_by_fork(self):
           pid = os.fork()
           if pid:
               self.log.info("Started process %d to run task", pid)
               return psutil.Process(pid)
           else:
               # Start a new process group
               os.setpgid(0, 0)
               import signal
   ````
   
   What we are using in Airflow we fork the process rather than start a new 
process by Popen by defauilt. This means that the child process will inherit 
opened handles from the parent process and it will "fork" - i.e. parent process 
will go one branch and the child process will start another branch. Both of 
them will have the "error_file" opened, and there is no Popen execution at all. 
   
   This allows to save seconds on starting a new task (there is no need to 
initialize new python interpreter, the task process will be a clone of the 
parent process with copy-on-write semantics when it comes to memory used, This 
basically means that it is almost instanteneous and there are no extra 
resources used.
   
   I believe (and please double check it) than we rely on the fact that the 
temporary error file remains open when the task is forked so that we do not 
re-open it again. If you close the error file before forking, the file will be 
closed and attempt to write to it will fail (at least I believe it will be like 
that).,




-- 
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]


Reply via email to