AyWa commented on a change in pull request #3337: [ZEPPELIN-4090] Ipython queue
performance
URL: https://github.com/apache/zeppelin/pull/3337#discussion_r323114745
##########
File path: python/src/main/resources/grpc/python/ipython_server.py
##########
@@ -94,69 +105,24 @@ def execute_worker():
t = threading.Thread(name="ConsumerThread", target=execute_worker)
with self._lock:
t.start()
- # We want to ensure that the kernel is alive because in case of
OOM or other errors
- # Execution might be stuck there:
- #
https://github.com/jupyter/jupyter_client/blob/master/jupyter_client/blocking/client.py#L32
- while t.is_alive() and self.isKernelAlive():
- while not text_queue.empty():
- output = text_queue.get()
- yield
ipython_pb2.ExecuteResponse(status=ipython_pb2.SUCCESS,
- type=ipython_pb2.TEXT,
- output=output)
- while not html_queue.empty():
- output = html_queue.get()
- yield
ipython_pb2.ExecuteResponse(status=ipython_pb2.SUCCESS,
- type=ipython_pb2.HTML,
- output=output)
- while not stderr_queue.empty():
- output = stderr_queue.get()
- yield ipython_pb2.ExecuteResponse(status=ipython_pb2.ERROR,
- type=ipython_pb2.TEXT,
- output=output)
- while not png_queue.empty():
- output = png_queue.get()
- yield
ipython_pb2.ExecuteResponse(status=ipython_pb2.SUCCESS,
- type=ipython_pb2.PNG,
- output=output)
- while not jpeg_queue.empty():
- output = jpeg_queue.get()
- yield
ipython_pb2.ExecuteResponse(status=ipython_pb2.SUCCESS,
- type=ipython_pb2.JPEG,
- output=output)
-
- # if kernel is not alive (should be same as thread is still alive),
means that we face
- # an unexpected issue.
- if not self.isKernelAlive() or t.is_alive():
- yield ipython_pb2.ExecuteResponse(status=ipython_pb2.ERROR,
- type=ipython_pb2.TEXT,
- output="Ipython kernel has
been stopped. Please check logs. It might be because of an out of memory
issue.")
- return
-
- while not text_queue.empty():
Review comment:
So the refactor I did, move all the logic in a single loop condition. So we
do not need to have multiple time the similar code (in the main loop and
outside of it)
It is line
https://github.com/apache/zeppelin/pull/3337/files#diff-9abe0e14503875941f3276b5f7743c46R113
until the queue is not empty we will not leave the main loop: `while
(t.is_alive() and self.isKernelAlive()) or not stream_reply_queue.empty():
`
Let me know if it makes sense or if I miss something
----------------------------------------------------------------
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:
us...@infra.apache.org
With regards,
Apache Git Services
