From 13fc7e5c41060e4b5bfe8b59562476aa9af5ab8d Mon Sep 17 00:00:00 2001 From: MinRK Date: Tue, 20 Dec 2011 12:26:06 -0800 Subject: [PATCH 1/5] cleanup connection files on notebook shutdown Kernels would not linger, but the KernelManagers are not garbage-collected on shutdown. This means that connection files for kernels still running at notebook shutdown would not be removed. Also disable the unnecessary (and actively unhelpful) SIGINT handler inherited from the original copy/paste from the qt app. --- IPython/frontend/html/notebook/notebookapp.py | 41 +++++++++++++------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/IPython/frontend/html/notebook/notebookapp.py b/IPython/frontend/html/notebook/notebookapp.py index 767903401..4a24538fb 100644 --- a/IPython/frontend/html/notebook/notebookapp.py +++ b/IPython/frontend/html/notebook/notebookapp.py @@ -303,9 +303,6 @@ class NotebookApp(BaseIPythonApplication): self.kernel_argv.append("--KernelApp.parent_appname='%s'"%self.name) def init_configurables(self): - # Don't let Qt or ZMQ swallow KeyboardInterupts. - signal.signal(signal.SIGINT, signal.SIG_DFL) - # force Session default to be secure default_secure(self.config) # Create a KernelManager and start a kernel. @@ -322,11 +319,9 @@ class NotebookApp(BaseIPythonApplication): # self.log is a child of. The logging module dipatches log messages to a log # and all of its ancenstors until propagate is set to False. self.log.propagate = False - - @catch_config_error - def initialize(self, argv=None): - super(NotebookApp, self).initialize(argv) - self.init_configurables() + + def init_webapp(self): + """initialize tornado webapp and httpserver""" self.web_app = NotebookWebApplication( self, self.kernel_manager, self.notebook_manager, self.log, self.webapp_settings @@ -339,7 +334,7 @@ class NotebookApp(BaseIPythonApplication): ssl_options = None self.web_app.password = self.password self.http_server = httpserver.HTTPServer(self.web_app, ssl_options=ssl_options) - if ssl_options is None and not self.ip: + if ssl_options is None and not self.ip and not (self.read_only and not self.password): self.log.critical('WARNING: the notebook server is listening on all IP addresses ' 'but not using any encryption or authentication. This is highly ' 'insecure and not recommended.') @@ -357,6 +352,23 @@ class NotebookApp(BaseIPythonApplication): else: self.port = port break + + @catch_config_error + def initialize(self, argv=None): + super(NotebookApp, self).initialize(argv) + self.init_configurables() + self.init_webapp() + + def cleanup_kernels(self): + """shutdown all kernels + + The kernels will shutdown themselves when this process no longer exists, + but explicit shutdown allows the KernelManagers to cleanup the connection files. + """ + self.log.info('Shutting down kernels') + km = self.kernel_manager + while km.kernel_ids: + km.kill_kernel(km.kernel_ids[0]) def start(self): ip = self.ip if self.ip else '[all ip addresses on your system]' @@ -371,15 +383,20 @@ class NotebookApp(BaseIPythonApplication): b = lambda : webbrowser.open("%s://%s:%i" % (proto, ip, self.port), new=2) threading.Thread(target=b).start() - - ioloop.IOLoop.instance().start() + try: + ioloop.IOLoop.instance().start() + except KeyboardInterrupt: + info("Interrupted...") + finally: + self.cleanup_kernels() + #----------------------------------------------------------------------------- # Main entry point #----------------------------------------------------------------------------- def launch_new_instance(): - app = NotebookApp() + app = NotebookApp.instance() app.initialize() app.start() From 15ca2aaa20c5e493d34a997619c02f5f7d369137 Mon Sep 17 00:00:00 2001 From: MinRK Date: Thu, 22 Dec 2011 22:38:57 -0800 Subject: [PATCH 2/5] add first_beat delay to notebook heartbeats Heartbeats start immediately, causing false heart failures on slow systems that can take a while to start kernel subprocesses. Also adds a 'flush' to the heartbeat callback (just like in IPython.parallel), to protect against server load being detected as heart failures. --- IPython/frontend/html/notebook/handlers.py | 8 ++++++-- IPython/frontend/html/notebook/kernelmanager.py | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/IPython/frontend/html/notebook/handlers.py b/IPython/frontend/html/notebook/handlers.py index f275d1864..9a420fc7a 100644 --- a/IPython/frontend/html/notebook/handlers.py +++ b/IPython/frontend/html/notebook/handlers.py @@ -18,6 +18,7 @@ Authors: import logging import Cookie +import time import uuid from tornado import web @@ -412,6 +413,7 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): return km = self.application.kernel_manager self.time_to_dead = km.time_to_dead + self.first_beat = km.first_beat kernel_id = self.kernel_id try: self.iopub_stream = km.create_iopub_stream(kernel_id) @@ -446,6 +448,7 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): self._kernel_alive = True def ping_or_dead(): + self.hb_stream.flush() if self._kernel_alive: self._kernel_alive = False self.hb_stream.send(b'ping') @@ -461,8 +464,9 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): self._kernel_alive = True self.hb_stream.on_recv(beat_received) - self._hb_periodic_callback = ioloop.PeriodicCallback(ping_or_dead, self.time_to_dead*1000) - self._hb_periodic_callback.start() + loop = ioloop.IOLoop.instance() + self._hb_periodic_callback = ioloop.PeriodicCallback(ping_or_dead, self.time_to_dead*1000, loop) + loop.add_timeout(time.time()+self.first_beat, self._hb_periodic_callback.start) self._beating= True def stop_hb(self): diff --git a/IPython/frontend/html/notebook/kernelmanager.py b/IPython/frontend/html/notebook/kernelmanager.py index c81664811..999bff6a8 100644 --- a/IPython/frontend/html/notebook/kernelmanager.py +++ b/IPython/frontend/html/notebook/kernelmanager.py @@ -195,7 +195,10 @@ class MappingKernelManager(MultiKernelManager): kernel_argv = List(Unicode) kernel_manager = Instance(KernelManager) + time_to_dead = Float(3.0, config=True, help="""Kernel heartbeat interval in seconds.""") + first_beat = Float(5.0, config=True, help="Delay (in seconds) before sending first heartbeat.") + max_msg_size = Integer(65536, config=True, help=""" The max raw message size accepted from the browser over a WebSocket connection. From 655bf1bacc088d1cbd670cba52e279c6196bb0f0 Mon Sep 17 00:00:00 2001 From: MinRK Date: Thu, 5 Jan 2012 20:05:32 -0800 Subject: [PATCH 3/5] prevent delayed heartbeat from starting on closed sessions --- IPython/frontend/html/notebook/handlers.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/IPython/frontend/html/notebook/handlers.py b/IPython/frontend/html/notebook/handlers.py index 9a420fc7a..f793408f5 100644 --- a/IPython/frontend/html/notebook/handlers.py +++ b/IPython/frontend/html/notebook/handlers.py @@ -458,7 +458,7 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): except: pass finally: - self._hb_periodic_callback.stop() + self.stop_hb() def beat_received(msg): self._kernel_alive = True @@ -466,12 +466,21 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): self.hb_stream.on_recv(beat_received) loop = ioloop.IOLoop.instance() self._hb_periodic_callback = ioloop.PeriodicCallback(ping_or_dead, self.time_to_dead*1000, loop) - loop.add_timeout(time.time()+self.first_beat, self._hb_periodic_callback.start) + loop.add_timeout(time.time()+self.first_beat, self._really_start_hb) self._beating= True + + def _really_start_hb(self): + """callback for delayed heartbeat start + + Only start the hb loop if we haven't been closed during the wait. + """ + if self._beating and not self.hb_stream.closed(): + self._hb_periodic_callback.start() def stop_hb(self): """Stop the heartbeating and cancel all related callbacks.""" if self._beating: + self._beating = False self._hb_periodic_callback.stop() if not self.hb_stream.closed(): self.hb_stream.on_recv(None) From 1385ef8ebb7c20c180adab00aab6933f66c536fd Mon Sep 17 00:00:00 2001 From: MinRK Date: Thu, 5 Jan 2012 21:12:58 -0800 Subject: [PATCH 4/5] log heartbeat failure in notebook --- IPython/frontend/html/notebook/handlers.py | 1 + 1 file changed, 1 insertion(+) diff --git a/IPython/frontend/html/notebook/handlers.py b/IPython/frontend/html/notebook/handlers.py index f793408f5..df3b65c0a 100644 --- a/IPython/frontend/html/notebook/handlers.py +++ b/IPython/frontend/html/notebook/handlers.py @@ -487,6 +487,7 @@ class IOPubHandler(AuthenticatedZMQStreamHandler): def kernel_died(self): self.application.kernel_manager.delete_mapping_for_kernel(self.kernel_id) + self.application.log.error("Kernel %s failed to respond to heartbeat", self.kernel_id) self.write_message( {'header': {'msg_type': 'status'}, 'parent_header': {}, From 87877e9a7f0e8a1069e1bc2dd0e472df7cc965c9 Mon Sep 17 00:00:00 2001 From: MinRK Date: Thu, 5 Jan 2012 21:53:32 -0800 Subject: [PATCH 5/5] explicit for-loop in cleanup_kernels makes single-iteration clearer than while loop, as reviewed by @fperez. --- IPython/frontend/html/notebook/notebookapp.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/IPython/frontend/html/notebook/notebookapp.py b/IPython/frontend/html/notebook/notebookapp.py index 4a24538fb..beb356b32 100644 --- a/IPython/frontend/html/notebook/notebookapp.py +++ b/IPython/frontend/html/notebook/notebookapp.py @@ -367,8 +367,9 @@ class NotebookApp(BaseIPythonApplication): """ self.log.info('Shutting down kernels') km = self.kernel_manager - while km.kernel_ids: - km.kill_kernel(km.kernel_ids[0]) + # copy list, since kill_kernel deletes keys + for kid in list(km.kernel_ids): + km.kill_kernel(kid) def start(self): ip = self.ip if self.ip else '[all ip addresses on your system]'