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

test(xsnap): Test xsnap close and terminate #2227

Merged
merged 1 commit into from
Jan 21, 2021
Merged

Conversation

kriskowal
Copy link
Member

This change adds tests to verify the behavior of graceful shutdown and graceless termination of an xsnap worker. Close signals to the xsnap worker that it will receive no further commands by closing the parentToChild file descriptor. Terminate sends a kill signal, and does not wait for the pending sequence of commands to flush (the “baton”).

Also added are some more debugging conveniences, like the threading of the Node.js writer stream and improved error messages. This also cleans up some of the debug vs non-debug switching.

@kriskowal kriskowal requested a review from dckc January 21, 2021 05:22
@kriskowal kriskowal force-pushed the xsnap-test-close-terminate branch from a1c0d6a to e174e49 Compare January 21, 2021 06:16
@dckc dckc merged commit 0b4c018 into master Jan 21, 2021
@dckc dckc deleted the xsnap-test-close-terminate branch January 21, 2021 17:46
@dckc dckc mentioned this pull request Jan 21, 2021
7 tasks
@dckc
Copy link
Member

dckc commented Jan 21, 2021

shoot... #2225 doesn't seem to work when rebased on top of this.

@kriskowal
Copy link
Member Author

kriskowal commented Jan 21, 2021 via email

@dckc
Copy link
Member

dckc commented Jan 21, 2021

A kernel level promise ends up pending rather than resolved.

That's the high level symptoms. Haven't diagnosed it more finely.

@kriskowal
Copy link
Member Author

The semantics of close changed such that it will wait until the commands in flight have finished. Changing close to terminate would determine whether that’s the root cause.

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.

2 participants