-
Notifications
You must be signed in to change notification settings - Fork 5.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow ?no_track_activity=1 to opt-out of activity tracking #4235
Conversation
In my thinking, it was actually deliberate that kernel shutdown counted as activity - I think of it as shut down inactive kernels after X seconds, and then shut down the server Y seconds after that. Maybe no-one else looks at it like that, though. :-) |
@@ -292,7 +292,6 @@ def shutdown_kernel(self, kernel_id, now=False): | |||
kernel._activity_stream = None | |||
self.stop_buffering(kernel_id) | |||
self._kernel_connections.pop(kernel_id, None) | |||
self.last_kernel_activity = utcnow() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was it necessary to remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessary, but a related change. This is the line that determines kernel shutdown == kernel activity even when the shutdown is caused by an idle timeout, where shutting down a kernel internally resets the idle clock (shutting down the kernel via the api still always resets the idle clock)
The implementation looks fine to me. |
I think that's a reasonable interpretation, it just doesn't happen to be mine. I initially thought this was fine, but as an operator of mybinder.org, it's been a little frustrating. There, I want to set a single timer for user goes away -> server shuts down. Whether they left kernels lying around doesn't influence when I want their server to be shutdown. I can't have that if we register idle-timeouts of kernels as activity. So my choice is either to set a more aggressive kernel timeout so the effective timeout of idle_shutdown + kernel timeout is ten minutes. But this would result in a different and shorter effective timeout for non-kernel-based sessions such as those just using terminals or rstudio behind nbrsessionproxy. |
Fair enough. Since this will change the meaning of existing config, can you write a release note about it? |
…counter allows external idle-culling scripts to avoid updating the activity counter
this causes idle-kernel shutdowns to restart the idle-shutdown timer user-requested shutdowns will still be tracked as api activity
6ee5bc2
to
c964d52
Compare
@takluyver good call. Added changelog note |
- add ``?no_track_activity=1`` argument to allow API requests | ||
to not be registered as activity (e.g. API calls by external activity monitors). | ||
- Kernels shutting down due to an idle timeout is no longer considered | ||
an activity-updating event. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks - can I ask you to make it explicit that the shutdown_no_activity_timeout
therefore starts counting from the last time a kernel is used, rather than starting from when the last kernel shuts down.
@minrk @takluyver Any other comments here? Otherwise merging it to master while trying to wrap up the 6.0 release. |
Fine by me. |
API requests with
?no_track_activity=1
will not update the API activity. This is useful for external activity tracking services (e.g. an idle culler for jupyterhub) to be able to poke around without themselves triggering updates to the activity counts. The main case affected right now is checking individual kernel activity via/api/kernels
.Additionally, don't track kernel shutdown as kernel activity. This causes idle-kernel shutdowns to restart the idle-shutdown timer, effectively doubling actual idle-shutdown time. User-requested shutdowns will still be tracked as api activity, so we don't need to track explicit shutdowns twice.