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

Add --idle-timeout to shutdown code-server when there isn't an active connection #1636

Closed
nhooyr opened this issue May 14, 2020 · 16 comments
Closed
Labels
feature New user visible feature
Milestone

Comments

@nhooyr
Copy link
Contributor

nhooyr commented May 14, 2020

If there hasn't been an active connection to code-server in X minutes, we should exit with status 0.

This would replace the heartbeat file (#1115) as a more friendly option.

cc @code-asher @antofthy

@nhooyr nhooyr added the enhancement Some improvement that isn't a feature label May 14, 2020
@antofthy
Copy link

antofthy commented May 16, 2020

No it does NOT replace the task of the heartbeat.

The heartbeat to an external file, allows an external monitor to shutdown the docker instance, not shutdown code-server itself. In fact the monitor itself may not even be running on the same machine in a multi-node docker environment.

There can be a lot more to the docker instance than code-server itself, such as an apache web server, or a java script node the user is developing in the isolated environment. A way to determine if the user is still active (via code-server) is an integral part of this, and would be a vastly harder thing to monitor without the availability of the heartbeat file.

In code-server v1 the file was actually a log that updated each time the web client requested an update, letting us know if the user still had a connected web server or not. While the web client was connected (even if the user was away from their terminal), the heartbeat would still update. So in a way heartbeat does not report 'user idle' status but 'connected' status.

So looking at it that way "heartbeat" is slightly different to the true meaning of "idle" as in the user is not really active (making changes, clicks, or typing).

@antofthy
Copy link

antofthy commented May 16, 2020

In a more general way. You may not actually want code-server to just shutdown, when not used for a certain length of time. It is only one possible action, in that situation. As mentioned in own case we want to shutdown the docker instance, and not code-server itself.

Now a -idle-timeout is still a good option... And I am certain many people would like to make use of such an option, especially when not using docker containers. But it is not the one needed in my setup!

TLDR... --idle-timeout is a good addition, but it is not a replacement for heartbeat.

@nhooyr
Copy link
Contributor Author

nhooyr commented May 17, 2020

@antofthy So instead of checking the heart beat file, you'd just check to see whether code-server or the container has exited successfully.

Can you not do that?

@antofthy
Copy link

antofthy commented May 18, 2020

Yes I could, but not as simply...

  • You can only look at processes from the same node in a docker cluster. As such the check would need to be done by each and every docker node.

  • You then have to parse the process list, which is always messy, and error prone thing to do.

  • You also lose information on how idle each container is, which can be important when looking for a time to upgrade the system in some way.

For example with heart beat I can do the following on the master node of cluster, for all containers (docker services)...

Name        Idle  Image        Node 
s1234567 13 secs  web-dev      elf1 
s7654321 18 secs  web-dev      elf1 
s1212121 13 secs  web-dev      elf  
Environments: 3 (3 active)

This shows at this moment I have 3 users using the system, all with a 'web-dev' code server environment (with apache, nodejs software) and they are all active (used system in last 5 minutes)
and only 2 docker nodes are in use at this time (elf, and elf1) That same program also is the one that will shutdown idle docker services after being idle for an hour, when called by cron with the appropriate options.

Time its been idle feedback is lost without heartbeat.

heartbeat is a LOT more versitile!

@antofthy
Copy link

Why the push to get rid of heartbeat. Is it causing some problem I don't know about?

@nhooyr
Copy link
Contributor Author

nhooyr commented May 18, 2020

No push, less features the better.

I understand your concerns, we'll keep both! 🎉

@nhooyr nhooyr added feature New user visible feature and removed enhancement Some improvement that isn't a feature labels Dec 8, 2020
@jsjoeio jsjoeio added this to the Backlog milestone Apr 29, 2021
@stale
Copy link

stale bot commented Oct 27, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no activity occurs in the next 5 days.

@stale stale bot added the stale label Oct 27, 2021
@stale stale bot closed this as completed Nov 1, 2021
@michaelosthege
Copy link

michaelosthege commented Nov 24, 2021

@nhooyr your last response sounded like you were going to add an --idle-timeout option, but the current release 3.12.0 only has the heartbeat mechanism, right?

For running code-server in a Docker container, the current situation is very inconvenient: A healthcheck can determine that the age of the heartbeat file exceeds a threshold, but since the main process in a Docker container (PID 1) ignores signals, a healthcheck script has no way to exit the code-server.

#!/bin/bash
now=$(date +%s)
fp=~/.local/share/code-server/heartbeat

# The heartbeat file does not exist before a browser session is connected,
# but we also want to exit if that never happens.
if [[ ! -f $fp ]]; then
    # No session yet.
    # Create the heartbeat file to mark the startup time.
    touch $fp
fi

heartbeat=$(date +%s -r $fp)

# NOTE: This was my original block and doesn't work 👇
#if (( (now-heartbeat) > 300 )); then
#    echo "Heartbeat file older than 5 minutes."
#    # Does not work, because Docker main processes ignore signals: kill -s SIGTERM 1
#    exit 1
#fi

if (( (now-heartbeat) > 300 )); then
    echo "Heartbeat older than 5 minutes. Terminating the code-server process."
    pkill -f code-server
    exit 1
else
    echo "Recent heartbeat detected."
    exit 0
fi

So IIUC there's no easy way to stop the Docker container "from the inside", but an external supervisor is needed. Please advise if there's a way.

@code-asher
Copy link
Member

code-asher commented Nov 24, 2021 via email

@michaelosthege
Copy link

[...] Could the healthcheck script kill code-server instead? Maybe with a pkill -f.

Yes, that works! I don't quite understand why, but fair enough. Thanks a lot!
I updated the code block above according to your solution.

@antofthy
Copy link

antofthy commented Jan 10, 2022

The only way to stop a docker container from the inside (unless special docker control is also given - but that is a special case) is to kill off whatever process the container is waiting on. That is the foreground 'START' process.

Basically that means you need specific knowledge of the set up so as to know what needs to be killed.

The 'heartbeat' file is very useful as if it is mounted from a persistent store (docker volume) it is accessible outside the docker container, where a global supervisory program can handle things.

However a -idle-timeout would be useful, but ONLY works to shutdown a docker container if the container is 'waiting' for code-server.

My problem now is I have one container with BOTH code-server and jupyter notebook running.
The latter does not have a heartbeat file (only a idle timeout), as to do idle properly I would need to idle check BOTH processes, which does not seem possible at this time.

Arrggghhh....

@jsjoeio
Copy link
Contributor

jsjoeio commented Jan 10, 2022

The latter does not have a heartbeat file (only a idle timeout), as to do idle properly I would need to idle check BOTH processes, which does not seem possible at this time.

I'll tag @code-asher to see if he has on ideas on how he would solve this

@eliliam
Copy link

eliliam commented Aug 11, 2023

Is this still a feature that's in the works?

@code-asher
Copy link
Member

code-asher commented Aug 11, 2023 via email

@antofthy
Copy link

It was added an an alternative to using 'heartbeat'.

The university I work for uses heartbeat to shutdown student virtual docker systems, when users stop using them. It works well, mostly because I can access the heartbeat outside the docker environment. Basically a cron script runs ever 10 minutes looking at active services, to see if heartbeat stopped updating for more than an 1 hour, then shotdown that users docker environment, no matter which machine in the cluster the docker service is running on. It also checks on 'control inputs' and even some web service connection (control server does not open/close connections). The "--idle-timeout" did not provide that level flexibility.

On the other hand there is a bug in that heartbeat stops updating if the user does NOT open new files for editing every so often, (just editing a single file or using console). So even it could use some improvement.

@doncadavona
Copy link

doncadavona commented Aug 14, 2023

We could use the healthz endpoint to check the status even from outside the VM or container where code-server is running:

As said in the docs' FAQ:

What is the healthz endpoint?

You can use the /healthz endpoint exposed by code-server to check whether code-server is running without triggering a heartbeat. The response includes a status (e.g., alive or expired) and a timestamp for the last heartbeat (the default is 0).

{
  "status": "alive",
  "lastHeartbeat": 1599166210566
}

I just saw this and currently exploring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New user visible feature
Projects
None yet
Development

No branches or pull requests

7 participants