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

Add restart intensity howto section to design principles doc #1289

Merged
merged 1 commit into from
Jan 26, 2017

Conversation

richcarl
Copy link
Contributor

The question "what values should I use for intensity/period in my supervisor" has been asked by both beginners and experienced people for a couple of decades, and it's always been hard to give a good answer. The Erlang documentation has nothing much helpful to say about this important choice. In this patch, I have tried to summarize how I think about these things nowadays, and make it accessible even for beginners. The OTP Design Principles section on Supervisor Maximum Restart Intensity seemed the best place for it.

Comments welcome.

apply a fix.</p>
<p>These choices depend a lot on your problem domain. If you don't
have real time monitoring and ability to fix problems quickly, for
example in in an embedded system, you might want to accept at most
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove one "in" from this line.

will be more reasonable.</p>
</item>
<item>
<p>If your application has nested supervisors, then the total
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to add a "Do not ..." sentence at the beginning of this bullet? (To make it the same pattern as the two bullets above.) Then probably change the naming of the list from "Common mistakes" to "Advice on how to avoid common mistakes" or similar.

times, which is probably excessive.</p>
</item>
</list></p>
<p>In general, a supervisor should have a lower intensity limit than
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this paragraph is tightly connected with the last bullet of the above list. Could it be moved into that bullet?

Copy link
Contributor

@sirihansen sirihansen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will add this to tests when the three line comments are followed up.

@richcarl
Copy link
Contributor Author

How about this version?

@sirihansen sirihansen added the testing currently being tested, tag is used by OTP internal CI label Jan 23, 2017
(before the top level supervisor gives up and terminates the
application) will be the product of the intensity values of all
the supervisors above the failing child process.</p>
<p>For example, If the top level allows 10 restarts, and the next
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a lower case 'i' in the 'If'.

@sirihansen
Copy link
Contributor

I have added this to our nightly build now. Please correct the upper case 'I' which I pointed out above, and I'll merge as soon as possible. Thanks for your contribution!

@richcarl
Copy link
Contributor Author

Fixed and squashed commits.

@sirihansen sirihansen merged commit 0101b6f into erlang:master Jan 26, 2017
@richcarl richcarl deleted the intensity-tuning-doc branch January 26, 2017 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants