tom-pytel commented on a change in pull request #125:
URL: https://github.com/apache/skywalking-python/pull/125#discussion_r661486113
##########
File path: skywalking/agent/__init__.py
##########
@@ -65,14 +76,40 @@ def __init():
__protocol = KafkaProtocol()
plugins.install()
+ __init_threading()
def __fini():
__protocol.report(__queue, False)
__queue.join()
+ __finished.set()
+
+
+def __fork_before():
+ if config.protocol != 'http':
+ logger.warning('fork() not currently supported with %s protocol' %
config.protocol)
Review comment:
I have tried a few more times, with upstream/master, and still bad
results. I did get one run where I got all 4 spans but the rest of the runs
were 3 spans with a couple of deadlocks. Apart from that, upstream/master can
not possibly run correctly in a multiprocessing scenario because on fork() no
other threads are duplicated in the child (like report or heartbeat), they need
to be explicitly recreated in a fork child (which I do in this PR).
I don't have time allocated now to look into the grpc issue but I do know
that http protocol in this PR works with fork() for sure. So how do you want to
proceed? I could remove that warning message if you want, or change it to
something a little less absolute like "fork() may not work correctly with grpc
protocol"? But in general this PR does not change anything about how grpc
worked before, just fixes the http protocol and adds restart of report and
heartbeat threads in fork() child. And also the celery plugin of course.
--
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]
