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

ctrl+c does not work when used with both --host and --no-detach #6585

Open
oliver-sanders opened this issue Jan 31, 2025 · 2 comments
Open
Labels
bug Something is wrong :( question Flag this as a question for the next Cylc project meeting. small
Milestone

Comments

@oliver-sanders
Copy link
Member

oliver-sanders commented Jan 31, 2025

If you are running a workflow remotely in no-detach mode, then running ctrl+c only kills the local process (which is essentially just doing cylc cat-log -m t at this point) but does not send the kill signal to the reinvoked remote process.

This makes perfect sense, the scheduler command was reinvoked on a remote host, ctrl+c is not going to kill that. However, this is inconsistent with the localhost behavior and the host config can be set in the global config so the user is not necessarily aware that the scheduler is being invoked remotely (caught me out!).

Reproducible Example

[me] $ cylc vip --host=other --no-detach myworkflow
...
^C
[me] $ cylc scan -f name
myworkflow
[me] $ echo 'confused!'

Options

  1. Don't allow this combination of options: play: --no-detach should infer --host=localhost #4886
  2. Capture KeyboardInterrupt and log a message explaining that the scheduler is still running.
  3. Capture KeyboardInterrupt and invoke a subprocess to run ssh <server> kill <pid> 🤮 .

I don't think that this combination of options is ever used intentionally so would lean towards option (1).

@oliver-sanders oliver-sanders added bug Something is wrong :( small labels Jan 31, 2025
@oliver-sanders oliver-sanders added this to the some-day milestone Jan 31, 2025
@oliver-sanders oliver-sanders added the question Flag this as a question for the next Cylc project meeting. label Jan 31, 2025
@hjoliver
Copy link
Member

hjoliver commented Feb 4, 2025

I guess the reasons we use --no-detach are:

  • (A) see the scheduler log in the terminal, without an extra cat-log step
  • (B) kill the scheduler in the terminal, without an extra cylc stop step

Those would be equally useful for remote runs, if we can could make it work properly.

So that suggests your 3. 🤮 is best!

If that's too nasty to implement, 2. is next best.

Number 1. seems overly draconian as we might still want to use (A) for convenience, even knowing that (B) doesn't work.

@oliver-sanders
Copy link
Member Author

oliver-sanders commented Feb 5, 2025

When we use --host and --no-detach in combination, Cylc isn't really doing what it says on the tin.

In fact we cannot reinvoke the command on the remote host without detaching it, we are essentially doing the same double-fork detach routine as we would in --detach mode, just using a remote process rather than a local fork.

So this option combo is a tad disingenuous. Assumptions you might make based on the docs are very incorrect, e.g:

# will it kill -> nope
cylc vip --host=abc --no-detach &
kill $!

# will it exit with the terminal -> nope
cylc vip --host=abc --no-detach &
kill -s sighup  $!

# will it report process stats -> nope
cylc vip --host=abc --no-detach &
cat /proc/pid/$!/meminfo

So this option combo is really complete bunk. If you actually want to run run the scheduler on a remote host in no-detach mode, there is only one way to do this:

ssh <hostname> cylc vip --no-detach

So on reflection:

  1. Is the only technically correct thing to do.
  2. Would need to be updated to print a message explaining that the scheduler is running detached.
  3. Is off the table IMO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( question Flag this as a question for the next Cylc project meeting. small
Projects
None yet
Development

No branches or pull requests

2 participants