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

remove overriding child_spec #43

Merged
merged 1 commit into from
Jan 26, 2024

Conversation

DohanKim
Copy link
Contributor

@DohanKim DohanKim commented Jan 25, 2024

We are overriding WalEx supervisor's child_spec without setting type key
which makes the type as default :worker instead of :supervisor
( https://hexdocs.pm/elixir/1.12/Supervisor.html#module-child-specification )

can check it by running

MyApp.Supervisor.which_children()

shows

[
  ...
  {WalEx.Supervisor, #PID<0.617.0>, :worker, [WalEx.Supervisor]},
  ...
]

This makes the whole app crash whenever any of the Event modules or other WalEx processes crashes.

we have choices of

  • adding type: :supervisor
  • removing overriding child_spec

I chose the latter as there is no significant reason to override the child_spec.

@cpursley
Copy link
Owner

Great find, thank you! I will review this weekend.

@DohanKim
Copy link
Contributor Author

Sorry, the original code doesn't necessarily crash the whole app as we are using supervisor hierarchy. But still, this can cause unexpected behavior when a direct child is terminated.

@cpursley
Copy link
Owner

Are you running on more than one node?

@DohanKim
Copy link
Contributor Author

@cpursley no, running only 1 node

@cpursley cpursley merged commit 2ae0086 into cpursley:master Jan 26, 2024
1 check passed
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