chamikaramj commented on issue #22533:
URL: https://github.com/apache/beam/issues/22533#issuecomment-1210018832
Update:
@drfloob confirmed that he's not able to confirm the fix anynmore as well.
So possibly something was not setup correctly when the fix was tried the first
time ?
Also, after reading subprocess docs.
Seems like the deadlock scenario is following.
* Do subprocess.Popen() to start a process while using stdout=PIPE or
stderr=PIPE.
* Do a wait() on that process.
* This could deadlock if the child process generates enough output to block
waiting for the OS buffer to clear.
But that's not what Beam is doing. Beam does following.
* Do a subprocess.Popen() to start a process while using
stdout=subprocess.PIPE and stderr=subprocess.STDOUT.
* Start a separate thread to read lines from the child process's stdout and
print those lines.
From documentation:
subprocess.PIPE:
Special value that can be used as the stdin, stdout or stderr argument to
[Popen]
(https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen) and
indicates that a pipe to the standard stream should be opened. Most useful with
[Popen.communicate()](https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen.communicate).
subprocess.STDOUT:
Special value that can be used as the stderr argument to [Popen]
(https://docs.python.org/3.8/library/subprocess.html#subprocess.Popen) and
indicates that standard error should go into the same handle as standard output.
Relavent code is here:
https://github.com/apache/beam/blob/4799828f75309fd494e14eab5af40559e9a140a3/sdks/python/apache_beam/utils/subprocess_server.py#L129
So I think Beam's implementation is correct and should not result in the
deadlock scenario mentioned in the documentation.
@drfloob and @gnossen do you agree ?
--
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]
