Adjust Idle culler settings and add internal culling #1133
Conversation
| @@ -107,6 +107,11 @@ module "jupyterhub" { | |||
| name = "dask-etc" | |||
| namespace = var.environment | |||
| kind = "configmap" | |||
| }, | |||
There was a problem hiding this comment.
I'd prefer if extra mounts was instead added within the module so that we can directly reference the config map name. There is a terraform concat function that can be used.
There was a problem hiding this comment.
Hi @costrouc, just to be sure... you are saying something like this
extra-mounts = merge(
var.extra-mounts,
{
"/etc/jupyter" = {
name = "server-idle-culling"
namespace = var.namespace
kind = "configmap"
}
}
)
on main.tf? I changed over merge, as concat only works for lists
viniciusdc
force-pushed
the
fix-974-idle_culler
branch
from
6bb418d
to
c896986
Compare
|
Finished testing with the config below:
Note that kernel/terminal shutdown represents user activity, so the total timeout for the process -- pod deletion, corresponds to the proximity sum of the kernel/terminal timeout + server (approximately)
|
|
I will soon add docs to this |
| @@ -30,7 +41,16 @@ resource "helm_release" "jupyterhub" { | |||
| shared-pvc = var.shared-pvc | |||
| conda-store-pvc = var.conda-store-pvc | |||
| conda-store-mount = var.conda-store-mount | |||
| extra-mounts = var.extra-mounts | |||
| extra-mounts = merge( | |||
There was a problem hiding this comment.
I think this is concat no? Merged will combine the dicts https://www.terraform.io/language/functions/merge.
There was a problem hiding this comment.
as var.extra-mounts is already a dict of the form
var.extra-mounts = {
"/dask-etc" = {}
}
and extra-mounts expect a dict for jsonencode, so we will be merging those dicts indeed, but only the new key idle-culler-... will be included, the values are maintained the same. Wich results in
extra-mounts = {
"/dask-etc" = {},
"/jupyter" = {}
}
Fixes | Closes | Resolves #974
This PR adds come extra configuration to the Jupyterhub idle culling system, right now it's enabled by default from jupyterhub values.yaml helm chart. This PR includes a new block of config for the culling system under the same yaml file.
As the user Jupyterlab server has an odd behavior when running alongside Jupyterhub, the WebSocket connection created for the window browser access to the user server generates false positives of the connected server (affecting the idle state) (see related topic here). To fix this issue, I've enabled the internal culling system for the Jupyterlab user server, which will cull idle kernels and terminals (see relevant information about the kernel inability to be correctly tracked sometimes, here)
As extra information about this general process, I recommend this comment here, explaining why using both internal and external idle culling services is a good path to go (see).
Changes introduced in this PR:
Types of changes
What types of changes does your PR introduce?
Put an
xin the boxes that applyTesting
Requires testing
In case you checked yes, did you write tests?
Yes
No
I think this would need extra testing implemented alongside Qhub, any ideas on this would be greatly welcomed, p.s, we can add a config extension to easily change the idle timeouts configuration -- starting point https://github.com/jupyterhub/jupyterhub-idle-culler/blob/main/tests/test_idle_culler.py
Documentation
Does your contribution include breaking changes or deprecations?
If so have you updated the documentation?
Further comments (optional)
Extra outputs from user pod, internal idle culling system:
c.AppServer....was used, the pod was terminated after the idle timeout was reached:Hub idle culling system when user log-outs: