From f827c30ef87befcd7f216faf258ac85b8af84685 Mon Sep 17 00:00:00 2001 From: Zach Sailer Date: Tue, 22 Mar 2022 15:37:57 -0700 Subject: [PATCH 1/2] wait to cleanup kernels after kernel is finished pending --- .../services/kernels/kernelmanager.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index 65582844cb..c9c315edf6 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -188,6 +188,17 @@ def cwd_for_path(self, path): os_path = os.path.dirname(os_path) return os_path + async def _remove_kernel_when_ready(self, kernel_id, kernel_awaitable): + try: + await super()._remove_kernel_when_ready(kernel_id, kernel_awaitable) + finally: + # Unlike its async sibling method in AsyncMappingKernelManager, removing the kernel_id + # from the connections dictionary isn't as problematic before the shutdown since the + # method is synchronous. However, we'll keep the relative call orders the same from + # a maintenance perspective. + self._kernel_connections.pop(kernel_id, None) + self._kernel_ports.pop(kernel_id, None) + async def start_kernel(self, kernel_id=None, path=None, **kwargs): """Start a kernel for a session and return its kernel_id. @@ -384,19 +395,12 @@ def shutdown_kernel(self, kernel_id, now=False, restart=False): self._check_kernel_id(kernel_id) self.stop_watching_activity(kernel_id) self.stop_buffering(kernel_id) - self._kernel_connections.pop(kernel_id, None) # Decrease the metric of number of kernels # running for the relevant kernel type by 1 KERNEL_CURRENTLY_RUNNING_TOTAL.labels(type=self._kernels[kernel_id].kernel_name).dec() self.pinned_superclass.shutdown_kernel(self, kernel_id, now=now, restart=restart) - # Unlike its async sibling method in AsyncMappingKernelManager, removing the kernel_id - # from the connections dictionary isn't as problematic before the shutdown since the - # method is synchronous. However, we'll keep the relative call orders the same from - # a maintenance perspective. - self._kernel_connections.pop(kernel_id, None) - self._kernel_ports.pop(kernel_id, None) async def restart_kernel(self, kernel_id, now=False): """Restart a kernel by kernel_id""" @@ -655,6 +659,4 @@ async def shutdown_kernel(self, kernel_id, now=False, restart=False): ret = await self.pinned_superclass.shutdown_kernel( self, kernel_id, now=now, restart=restart ) - self._kernel_connections.pop(kernel_id, None) - self._kernel_ports.pop(kernel_id, None) return ret From 7f30adc6ec3f87de8817c067620fdffe6c342f5e Mon Sep 17 00:00:00 2001 From: Zach Sailer Date: Wed, 23 Mar 2022 07:56:55 -0700 Subject: [PATCH 2/2] remove kernel ports/connections conditionally on kernel shutdown success --- jupyter_server/services/kernels/kernelmanager.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/jupyter_server/services/kernels/kernelmanager.py b/jupyter_server/services/kernels/kernelmanager.py index c9c315edf6..461b8b4351 100644 --- a/jupyter_server/services/kernels/kernelmanager.py +++ b/jupyter_server/services/kernels/kernelmanager.py @@ -189,15 +189,9 @@ def cwd_for_path(self, path): return os_path async def _remove_kernel_when_ready(self, kernel_id, kernel_awaitable): - try: - await super()._remove_kernel_when_ready(kernel_id, kernel_awaitable) - finally: - # Unlike its async sibling method in AsyncMappingKernelManager, removing the kernel_id - # from the connections dictionary isn't as problematic before the shutdown since the - # method is synchronous. However, we'll keep the relative call orders the same from - # a maintenance perspective. - self._kernel_connections.pop(kernel_id, None) - self._kernel_ports.pop(kernel_id, None) + await super()._remove_kernel_when_ready(kernel_id, kernel_awaitable) + self._kernel_connections.pop(kernel_id, None) + self._kernel_ports.pop(kernel_id, None) async def start_kernel(self, kernel_id=None, path=None, **kwargs): """Start a kernel for a session and return its kernel_id.