[API server] cleanup executor processes on shutdown

[aylei]

close #4894

SIGTERM handling is relevant not only because sky cancel, but also because Kubernetes sends SIGTERM to restart failed server when liveness probe fails. Besides, some frameworks like openkruise on k8s also send SIGTERM to restart the Pod in-place when updating the container image.

This also closes #4856 via properly interrupting executors.

Future works:

1. unify the cleanup in sky api stop to server-side, then sky api stop only sends SIGTERM for consistency
2. handle SIGKILL, typically sent by OOM killer. This requires setting up some local state I think, either a monitor process or a filelock that hints the restarted server there is a uncleaned state and some cleanup work should be taken

@cg505 @Michaelvll PTAL if this makes sense

Tested (run the relevant ones):

• Code formatting: bash format.sh
• Manually tested SIGTERM a server with and without --foreground
• All smoke tests: pytest tests/test_smoke.py
• Relevant individual smoke tests: pytest tests/test_smoke.py::test_fill_in_the_name
• Backward compatibility tests: conda deactivate; bash -i tests/backward_compatibility_tests.sh

[cg505]

Seems that ideally we shouldn't use KeyboardInterrupt for the worker SIGTERM. It makes sense for request execution since all the core code is designed to work with KeyboardInterrupt, but here we can fully control the exception type we throw and catch. We could use a custom exception.
Super low priority, probably better to just merge this. Could add a TODO comment.

[aylei]

Sure! My original intent is to handle Ctrl-C (in --foreground mode) and SIGTERM from parent process in a uniform way, but agree it is better to use a distinct exception for SIGTERM

[cg505]

Thanks for the change, I think this is good. @Michaelvll should review as well.

2. handle SIGKILL, typically sent by OOM killer. This requires setting up some local state I think, either a monitor process or a filelock that hints the restarted server there is a uncleaned state and some cleanup work should be taken

Another option could be that the worker itself periodically checks if the api server PID is still alive, and exits if not. This could avoid needing a separate monitor process. Just an idea, not sure what's the best option.

[Michaelvll]

Thanks @aylei! LGTM.

[Michaelvll]

Should this be under tests/unit_tests/sky/utils/?

[aylei]

Ah, good catch

[Michaelvll]

/smoke-test -k test_multi_echo
/smoke-test -k test_cancel_pytorch
/smoke-test -k test_cancel_aws --aws
/smoke-test -k test_job_queue

[aylei]

/smoke-test -k test_multi_echo
/smoke-test -k test_cancel_pytorch
/smoke-test -k test_cancel_aws --aws
/smoke-test -k test_job_queue

[aylei]

/smoke-test -k test_job_queue