Avoid 'weaver multi' zombie main components. #644
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Recall that when you run `weaver multi deploy`, you start a `weaver
multi deploy` process that runs a bunch of remote weavelets in
subprocesses. The weavelets communicate with the `weaver multi deploy`
process over a set of pipes. Here's an example of what `top` shows when
deploying the collatz app:
```
-bash
└─ cmd/weaver/weaver multi deploy weaver.toml
├─ examples/collatz/collatz
├─ examples/collatz/collatz
├─ examples/collatz/collatz
├─ examples/collatz/collatz
├─ examples/collatz/collatz
└─ examples/collatz/collatz
```
If you `kill` the `weaver multi deploy` process, it doesn't kill the
child weavelet processes, at least not directly. However, killing the
`weaver multi deploy` process closes the pipes to the weavelets. This
should cause the weavelet processes to exit.
This was working correctly for all weavelets not running the main
component. weavelets running the main component blocked on the user
provided lambda. If this lambda did something like run an HTTP server,
then the process wouldn't exit, even if the underlying weavelet was
broken. This led to a bunch of zombie processes lingering around
forever.
This PR fixes the bug by exiting whenever the weavelet encounters a
broken pipe.
Also note that `examples/examples_test.go` runs into this zombie
behavior. So, if you've been running `./dev/build_and_test.sh`, you
might to run `ps aux | grep weaver` to see if you have a bunch of
zombies.
spetrovic77
approved these changes
Sep 28, 2023
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Recall that when you run
weaver multi deploy, you start aweaver multi deployprocess that runs a bunch of remote weavelets in subprocesses. The weavelets communicate with theweaver multi deployprocess over a set of pipes. Here's an example of whattopshows when deploying the collatz app:If you
killtheweaver multi deployprocess, it doesn't kill the child weavelet processes, at least not directly. However, killing theweaver multi deployprocess closes the pipes to the weavelets. This should cause the weavelet processes to exit.This was working correctly for all weavelets not running the main component. weavelets running the main component blocked on the user provided lambda. If this lambda did something like run an HTTP server, then the process wouldn't exit, even if the underlying weavelet was broken. This led to a bunch of zombie processes lingering around forever.
This PR fixes the bug by exiting whenever the weavelet encounters a broken pipe.
Also note that
examples/examples_test.goruns into this zombie behavior. So, if you've been running./dev/build_and_test.sh, you might to runps aux | grep weaverto see if you have a bunch of zombies.