Skip to content
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

Merged
merged 5 commits into from
Jun 25, 2019

Conversation

minrk
Copy link
Member

@minrk minrk commented Nov 26, 2018

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.

@minrk
Copy link
Member Author

minrk commented Nov 26, 2018

cf jupyterhub/jupyterhub#2251

@takluyver
Copy link
Member

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()
Copy link
Member

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?

Copy link
Member Author

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)

@takluyver
Copy link
Member

The implementation looks fine to me.

@minrk
Copy link
Member Author

minrk commented Nov 29, 2018

shut down inactive kernels after X seconds, and then shut down the server Y seconds after that

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.

@takluyver takluyver modified the milestones: 5.8, 6.0 Jan 31, 2019
@takluyver
Copy link
Member

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
@minrk
Copy link
Member Author

minrk commented Feb 11, 2019

@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.
Copy link
Member

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.

@lresende
Copy link
Member

@minrk @takluyver Any other comments here? Otherwise merging it to master while trying to wrap up the 6.0 release.

@takluyver
Copy link
Member

Fine by me.

@takluyver takluyver merged commit efc0f00 into jupyter:master Jun 25, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants