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

tests: fix long shutdown times for the webhook_receiver container #5458

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Dec 13, 2022

Currently, server.py runs as PID 1, which means that it won't be terminated by a SIGTERM signal unless it explicitly handles it (which it doesn't). So when Docker tries to shut the container down, it sends the server a SIGTERM, which gets ignored, and then sits there for 10 seconds before sending it a SIGKILL.

To work around this, enable the built-in Docker init program, which forwards signals to the Python server. Since the Python server is no longer PID 1, SIGTERM will now shut it down immediately.

The init option is supported starting from the Compose format version 3.7, so bump the version.

Motivation and context

I noticed that webhook_receiver takes the longest of all test containers to shut down, even though it's a trivial server.

How has this been tested?

time docker container stop test_webhook_receiver_1

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

Currently, `server.py` runs as PID 1, which means that it won't be
terminated by a `SIGTERM` signal unless it explicitly handles it (which it
doesn't). So when Docker tries to shut the container down, it sends the
server a `SIGTERM`, which gets ignored, and then sits there for 10 seconds
before sending it a `SIGKILL`.

To work around this, enable the built-in Docker init program, which forwards
signals to the Python server. Since the Python server is no longer PID 1,
`SIGTERM` will now shut it down immediately.

The `init` option is supported starting from the Compose format version 3.7,
so bump the version.
Copy link
Contributor

@sizov-kirill sizov-kirill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@SpecLad
Copy link
Contributor Author

SpecLad commented Dec 16, 2022

@nmanovic Mind merging this?

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I believe it is the simplest fix for the issue.

@nmanovic nmanovic merged commit 7b13216 into cvat-ai:develop Dec 16, 2022
@SpecLad SpecLad deleted the webhook-receiver-shutdown branch December 16, 2022 12:25
mikhail-treskin pushed a commit to retailnext/cvat that referenced this pull request Jul 1, 2023
…at-ai#5458)

Currently, `server.py` runs as PID 1, which means that it won't be
terminated by a `SIGTERM` signal unless it explicitly handles it (which
it doesn't). So when Docker tries to shut the container down, it sends
the server a `SIGTERM`, which gets ignored, and then sits there for 10
seconds before sending it a `SIGKILL`.

To work around this, enable the built-in Docker init program, which
forwards signals to the Python server. Since the Python server is no
longer PID 1, `SIGTERM` will now shut it down immediately.

The `init` option is supported starting from the Compose format version
3.7, so bump the version.
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.

None yet

3 participants