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

controller: finish remote controller when all session finished #2010

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ktock
Copy link
Collaborator

@ktock ktock commented Aug 21, 2023

#1641

This PR tries to implement automatic cleanup of the remote controller suggested by @jedevc on #1641 .

Most of the items listed in that issue seem already implemented through the previous works. So this PR mainly focuses on implementing automatic exiting of the remote server when there is no session (caa83a6), which hasn't been implemented. This is implemented by periodically (every 10s) checking the number of sessions managed on the server. If there is no session, sever automatically exits.

Please note that this PR doesn't implement automatic finishing of a session after a deadline (mainly because I'm not sure how long the deadline should be).

example

Session can be finished using disconnect.

(buildx) list
ID				CURRENT_SESSION
n9bsbbg5vhyl2chcxzre0z0l6	true
(buildx) disconnect
(buildx) list
ID	CURRENT_SESSION

On another terminal, we can see the server automatically exits after 10s of the disconnection.

$ ps auxww | grep _INTERNAL_SERVE
ktock     968909  1.1  0.3 1726976 51412 ?       Ssl  14:23   0:00 /tmp/out/buildx _INTERNAL_SERVE
ktock     969048  0.0  0.0  10096   724 pts/10   S+   14:23   0:00 grep --color=auto _INTERNAL_SERVE

...(10s after)...

$ ps auxww | grep _INTERNAL_SERVE
ktock     969051  0.0  0.0  10096   720 pts/10   S+   14:24   0:00 grep --color=auto _INTERNAL_SERVE

Implementation details for #1641

The following shows details about each item listed in #1641 :

  • Processes are terminated up 1. when they exit, or 2. when their parent session terminates
  • A session should terminate 1. when it is explicitly terminated by the user, 2. when its parent controller is terminated (for local, when the calling process ends, for remote, when the server terminates), or 3. after a deadline during which no user interaction has occurred
    • The deadline is useful, so that a user can run a build, watch it fail, and then connect to the session to investigate and invoke, without needing to re-trigger the build.
  • 1. when it is explicitly terminated by the user: Implemented by disconnect command on the monitor. (this PR a fix for local controller: eb2b88b)
  • 2. when its parent controller is terminated (for local, when the calling process ends, for remote, when the server terminates):
  • 3. after a deadline during which no user interaction has occurred: not implemented. I'm not sure about how long the timeout should be.
  • In addition to the above, when the monitor finishes (e.g. by Ctrl-D), it automatically finishes the session via Disconnect API.
  • The remote server should terminate itself when 1. the user requests it to using kill, or 2. it has no more sessions (so that after the deadlines of all the sessions have expired, we can cleanly exit).
  • 1. the user requests it to using kill: Implemented by kill command on the monitor.
  • 2. it has no more sessions: Implemented by this PR (caa83a6).

@ktock ktock changed the title debug: finish remote controller when all session finished controller: finish remote controller when all session finished Aug 21, 2023
@tonistiigi tonistiigi requested a review from jedevc October 11, 2023 23:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant