[GitHub] [zeppelin] AyWa commented on a change in pull request #3337: [ZEPPELIN-4090] Ipython queue performance

[GitBox <gi...@apache.org>]

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():
    `
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services