Skip to content

Commit

Permalink
Culling: ensure last_activity attr exists before use (#5355)
Browse files Browse the repository at this point in the history
KernelManager's last_activity attribute is added following the instance's
creation.  Culling (independently) uses this attribute and should ensure
it exists prior to its use, else skip the culling check for that instance.

Fixes #5345
  • Loading branch information
kevin-bates authored May 25, 2020
1 parent 4da22ce commit 6e9256b
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 3 deletions.
6 changes: 3 additions & 3 deletions notebook/services/kernels/kernelmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,9 @@ async def cull_kernel_if_idle(self, kernel_id):
except KeyError:
return # KeyErrors are somewhat expected since the kernel can be shutdown as the culling check is made.

self.log.debug("kernel_id=%s, kernel_name=%s, last_activity=%s",
kernel_id, kernel.kernel_name, kernel.last_activity)
if kernel.last_activity is not None:
if hasattr(kernel, 'last_activity'): # last_activity is monkey-patched, so ensure that has occurred
self.log.debug("kernel_id=%s, kernel_name=%s, last_activity=%s",
kernel_id, kernel.kernel_name, kernel.last_activity)
dt_now = utcnow()
dt_idle = dt_now - kernel.last_activity
# Compute idle properties
Expand Down
47 changes: 47 additions & 0 deletions notebook/services/kernels/tests/test_kernels_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import sys
import time

from requests import HTTPError
from traitlets.config import Config

from tornado.httpclient import HTTPRequest
Expand Down Expand Up @@ -234,3 +235,49 @@ class KernelFilterTest(NotebookTestBase):
# Sanity check verifying that the configurable was properly set.
def test_config(self):
self.assertEqual(self.notebook.kernel_manager.allowed_message_types, ['kernel_info_request'])


class KernelCullingTest(NotebookTestBase):
"""Test kernel culling """

@classmethod
def get_argv(cls):
argv = super(KernelCullingTest, cls).get_argv()

# Enable culling with 2s timeout and 1s intervals
argv.extend(['--MappingKernelManager.cull_idle_timeout=2',
'--MappingKernelManager.cull_interval=1',
'--MappingKernelManager.cull_connected=False'])
return argv

def setUp(self):
self.kern_api = KernelAPI(self.request,
base_url=self.base_url(),
headers=self.auth_headers(),
)

def tearDown(self):
for k in self.kern_api.list().json():
self.kern_api.shutdown(k['id'])

def test_culling(self):
kid = self.kern_api.start().json()['id']
ws = self.kern_api.websocket(kid)
model = self.kern_api.get(kid).json()
self.assertEqual(model['connections'], 1)
assert not self.get_cull_status(kid) # connected, should not be culled
ws.close()
assert self.get_cull_status(kid) # not connected, should be culled

def get_cull_status(self, kid):
culled = False
for i in range(15): # Need max of 3s to ensure culling timeout exceeded
try:
self.kern_api.get(kid)
except HTTPError as e:
assert e.response.status_code == 404
culled = True
break
else:
time.sleep(0.2)
return culled

0 comments on commit 6e9256b

Please sign in to comment.