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

OTP-ify tinymq application. #3

Merged
merged 1 commit into from
Aug 31, 2012
Merged

Conversation

robertoaloi
Copy link
Contributor

  • Implement application behaviour
  • Use env variable for newage parameter
  • Utilize simple_one_for_one supervisor

According to the current implementation, the controller acts as some sort of "supervisor" for the channel_sup. A channel_sup is created for each channel. Is this intentional? Wouldn't it make sense to have a sub-tree with one channel sup and several channel?

@evanmiller
Copy link
Contributor

Thanks for the pull request! This looks good, so I'll merge it in.

What's wrong with one-to-one channel supervision? Are you worried about memory?

evanmiller added a commit that referenced this pull request Aug 31, 2012
OTP-ify tinymq application.
@evanmiller evanmiller merged commit 0cbf918 into ChicagoBoss:master Aug 31, 2012
@robertoaloi
Copy link
Contributor Author

Simply wondering why you need of a supervisor for each channel. Couldn't you have a unique supervisor monitoring multiple channels? That would make you spare a bunch of processes. I guess I'm thinking about a root supervisor having two children: the controller and a channel supervisor. The channel supervisor would be the simple_one_for_one, supervising multiple channels. Just an idea.

@evanmiller
Copy link
Contributor

That makes sense to me, and seems to be a cleaner supervision design than what's currently implemented. I'll be happy to merge in a pull request with that change (if you get around to it).

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