<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Sep 3, 2014 at 1:50 PM, Siri Hansen <span dir="ltr"><<a href="mailto:erlangsiri@gmail.com" target="_blank">erlangsiri@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hello!<div><br><div><div>It looks like we have reached consensus regarding the default values :) Thanks again for all input!</div>
<div><br></div><div>Over the summer we have had some internal reviews of the documentation and I got some feedback on the naming. Specifically, questions are raised about the keys 'name', 'intensity' and 'period'. With the old tuple format, these three values are named Id, MaxR and MaxT respectively, and the questions are if they should be changed at all and if so, are the suggested names "the best"...</div>
<div><br></div><div>Personally, I think that changing Id to 'name' might not be necessary. Partly because this value is discussed as the "child identifier" everywhere in the documentation and partly because the type definition of it is term() (and not atom() or string()). So my suggestion is to change this key to 'id'.</div>
<div><br></div><div>The reasoning about 'intensity'/MaxR and 'period'/MaxT is more complex, I think. I agree that the old variable names (at least MaxT), are not super good. One reason for keeping to something similar to this is, however, that these names are used when describing the mechanism of restart escalation in some of the erlang books that already exist. And if you are familiar with the old format, then switching to the new format will of course be easier if the key names are easy to map. But on the other hand, if we could find really good names to use instead, then maybe it would be worth the change in order to ease the introduction for newcomers. Again personally, I am not sure that 'intensity' and 'period' are so much better than the old names...</div>
<div><br></div><div>What we are talking about here is some kind of "restart frequency", although it is not really a frequency either - at least not a constant frequency. It's the limit for the number of restarts within a time window... Are the names max_restarts or restart_limit better than intensity. Is period ok, or time_frame, time_window ... ? Or should we stick to max_time to keep close to the old names?</div>
<div><br></div><div>Would it make sense to put both values into a tuple using only one key (restart_frequency, ...?) in the map?<br></div><div><br></div></div></div><div>So many questions... naming is hard! Any thoughts?</div>
<div><br></div><div>/siri</div></div></blockquote><div><br></div><div>Would it make sense to have a single property "max_restart_frequency" whose value is a tuple {R, T} ?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote">2014-07-04 12:13 GMT+02:00 Siri Hansen <span dir="ltr"><<a href="mailto:erlangsiri@gmail.com" target="_blank">erlangsiri@gmail.com</a>></span>:<div>
<div class="h5"><br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr">I have pushed the current status of this job to github. Comments still welcome!<div>
/siri</div><div><br>
</div><div><a href="https://github.com/sirihansen/otp/compare/erlang:980d7fa32128d43d36fb9527df1967eff801be9c...siri/sup-spec-maps/OTP-11043" target="_blank">https://github.com/sirihansen/otp/compare/erlang:980d7fa32128d43d36fb9527df1967eff801be9c...siri/sup-spec-maps/OTP-11043</a><br>
</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">2014-07-03 20:24 GMT+02:00 DeadZen <span dir="ltr"><<a href="mailto:deadzen@deadzen.com" target="_blank">deadzen@deadzen.com</a>></span>:<div><div>
<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+1 on the latest revision<br>
<div><div><br>
On Thu, Jul 3, 2014 at 9:57 AM, Siri Hansen <<a href="mailto:erlangsiri@gmail.com" target="_blank">erlangsiri@gmail.com</a>> wrote:<br>
> Hello,<br>
><br>
> thanks for the feedback! We have decided to take your comments into account<br>
> and change the default values as follows:<br>
><br>
> Intensity/Perior: 1/5<br>
> Shutdown for worker: 5000<br>
> Shutdown for supervisor: infinity<br>
><br>
> The reasoning behind it is that<br>
><br>
> 1. as a few of you commented, it is probably not a good idea to have a<br>
> default without restarts, as restarts is a very important property of the<br>
> supervisor tree. Using 1 restart in 5 seconds was a compromise between my<br>
> first intention of "being extreme" and the suggestions from the list... And<br>
> also - if a process can not be restarted on the first attempt, then maybe it<br>
> is not so great a chance that it will succeed on the second attempt either.<br>
> And 5 seconds is a very good number!!<br>
><br>
> 2. to avoid the confusion of why the terminate function is not executed, we<br>
> decided not to use 'brutal_kill'. Which number to use is probably not that<br>
> important, so we chose 5000 - which we all know is proven to be "the best<br>
> timeout value" ;)<br>
><br>
> 3. the recommended shutdown value for supervisors is 'infinity', and I think<br>
> that should be reflected in the default. Having the same (integer) value as<br>
> for workers can give very strange results.<br>
><br>
> Further comments are still welcome... :)<br>
><br>
> /siri<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
> 2014-07-01 14:43 GMT+02:00 Daniel Luna <<a href="mailto:daniel@lunas.se" target="_blank">daniel@lunas.se</a>>:<br>
><br>
>> I have to mention that I think brutal_kill is a really bad choice for a<br>
>> default.<br>
>><br>
>> It breaks the expectation in Erlang that supervisors can never crash. An<br>
>> expectation that is in the core of what makes Erlang the great language that<br>
>> it is.<br>
>><br>
>> Dangling process trees are not fun to deal with (and don't happen on a VM<br>
>> crash).<br>
>><br>
>> To preserve the correctness and trust of supervision trees the default<br>
>> should be either infinity or depend on the child type.<br>
>><br>
>> For worker children brutal_kill is fine. For supervisor children infinity<br>
>> is the only sensible choice.<br>
>><br>
>> On Jul 1, 2014 7:06 AM, "Loïc Hoguin" <<a href="mailto:essen@ninenines.eu" target="_blank">essen@ninenines.eu</a>> wrote:<br>
>>><br>
>>> Just wanted to say I agree with everything José said, except 3. the<br>
>>> different defaults for different process types. It makes things too hard to<br>
>>> keep track of.<br>
>>><br>
>>> I think brutal_kill is a great choice because it makes it easier for the<br>
>>> user to stumble upon cold-start issues when restarting their<br>
>>> application/release. The VM may crash so there's never a guarantee to run<br>
>>> shutdown functions anyway, and too few people forget to test cases when<br>
>>> these didn't run, leading to potential production disasters.<br>
>>><br>
>>> On 06/30/2014 04:37 PM, José Valim wrote:<br>
>>>><br>
>>>> I have some feedback based on my experience in writing and teaching<br>
>>>> Erlang/Elixir.<br>
>>>><br>
>>>> 1. I would always require the supervision strategy. When I am<br>
>>>> prototyping, it is usually hard for me to reason about the intensity and<br>
>>>> restarts, but I can always reason about the supervision strategy. All<br>
>>>> teaching materials I know of also discuss the supervision right away, so<br>
>>>> having it explicit shouldn't hurt those cases.<br>
>>>><br>
>>>> For such cases, maybe we should use the format of {{strategy(), map()},<br>
>>>> [children()]} (which is similar to what we have today).<br>
>>>><br>
>>>> 2. I also agree with Richard. Having a default of 0 restarts may cause<br>
>>>> confusion and forcing people to define them won't help when they still<br>
>>>> have no idea of what values they should use (because it is prototyping /<br>
>>>> early deploys). I usually fine tune those values based on the production<br>
>>>> system reports so having saner defaults, like Ulf's, may make more<br>
>>>> sense.<br>
>>>><br>
>>>> 3. The child_spec defaults are great and similar to the ones we use in<br>
>>>> Elixir with the only difference being the shutdown value. We have a<br>
>>>> default of 5000, which is the one usually recommended, but we set it to<br>
>>>> "infinity" if the type is a "supervisor" (which is the OTP docs<br>
>>>> recommendation if I remember correctly).<br>
>>>><br>
>>>> Regarding 3., if the recommendation and the consensus in actual systems<br>
>>>> is to use brutal_kill for shutdown, I will gladly change Elixir to make<br>
>>>> it consistent with OTP defaults.<br>
>>>><br>
>>>> Thanks for opening the changes for discussion!<br>
>>>><br>
>>>> *José Valim*<br>
>>>> <a href="http://www.plataformatec.com.br" target="_blank">www.plataformatec.com.br</a> <<a href="http://www.plataformatec.com.br/" target="_blank">http://www.plataformatec.com.br/</a>><br>
>>>> Skype: jv.ptec<br>
>>>> Founder and Lead Developer<br>
>>>><br>
>>>><br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> erlang-patches mailing list<br>
>>>> <a href="mailto:erlang-patches@erlang.org" target="_blank">erlang-patches@erlang.org</a><br>
>>>> <a href="http://erlang.org/mailman/listinfo/erlang-patches" target="_blank">http://erlang.org/mailman/listinfo/erlang-patches</a><br>
>>>><br>
>>><br>
>>> --<br>
>>> Loïc Hoguin<br>
>>> <a href="http://ninenines.eu" target="_blank">http://ninenines.eu</a><br>
>>> _______________________________________________<br>
>>> erlang-patches mailing list<br>
>>> <a href="mailto:erlang-patches@erlang.org" target="_blank">erlang-patches@erlang.org</a><br>
>>> <a href="http://erlang.org/mailman/listinfo/erlang-patches" target="_blank">http://erlang.org/mailman/listinfo/erlang-patches</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> erlang-patches mailing list<br>
> <a href="mailto:erlang-patches@erlang.org" target="_blank">erlang-patches@erlang.org</a><br>
> <a href="http://erlang.org/mailman/listinfo/erlang-patches" target="_blank">http://erlang.org/mailman/listinfo/erlang-patches</a><br>
><br>
</div></div></blockquote></div></div></div><br></div>
</blockquote></div></div></div><br></div></div>
<br>_______________________________________________<br>
erlang-patches mailing list<br>
<a href="mailto:erlang-patches@erlang.org">erlang-patches@erlang.org</a><br>
<a href="http://erlang.org/mailman/listinfo/erlang-patches" target="_blank">http://erlang.org/mailman/listinfo/erlang-patches</a><br>
<br></blockquote></div><br></div></div>