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

update real world usage connection pool to use supervisor behavior #276

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bubiche
Copy link

@bubiche bubiche commented Jan 13, 2025

workerB

My setup requires the children defined before MyApp.Redix to be started before its child_spec can run (need to read some data from somewhere else) and I struggled for a bit with it so I think it might be worth updating the document to a more fool-proof default.

I think use Supervisor is better than manually setup through child_spec since it follows OTP convention and is easier to debug, it also means whichever is specified before MyApp.Redix in the main supervisor tree is actually started before we do anything with MyApp.Redix.

@whatyouhide
Copy link
Owner

Hi @bubiche, thanks for the issue.

it follows OTP convention

I don't think it necessarily does? Module-based supervisors vs Supervisor.start_links are not an OTP convention as far as I’m aware.

easier to debug

How?

it also means whichever is specified before MyApp.Redix in the main supervisor tree is actually started before we do anything with MyApp.Redix.

This is the same with the previous approach, as Supervisor.start_link/2 will still block until its children get started.

My setup requires the children defined before MyApp.Redix to be started before its child_spec can run

In this case yes you will need to "init" the supervisor after other things have been started, but it seems like a less common case. Can you elaborate with a more concrete example? 🙃

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.

3 participants