[erlang-patches] allow use of proplists in supervisor start specs

[Siri Hansen]

Hello,

thanks for the feedback! We have decided to take your comments into account
and change the default values as follows:

Intensity/Perior: 1/5
Shutdown for worker: 5000
Shutdown for supervisor: infinity

The reasoning behind it is that

1. as a few of you commented, it is probably not a good idea to have a
default without restarts, as restarts is a very important property of the
supervisor tree. Using 1 restart in 5 seconds was a compromise between my
first intention of "being extreme" and the suggestions from the list... And
also - if a process can not be restarted on the first attempt, then maybe
it is not so great a chance that it will succeed on the second attempt
either. And 5 seconds is a very good number!!

2. to avoid the confusion of why the terminate function is not executed, we
decided not to use 'brutal_kill'. Which number to use is probably not that
important, so we chose 5000 - which we all know is proven to be "the best
timeout value" ;)

3. the recommended shutdown value for supervisors is 'infinity', and I
think that should be reflected in the default. Having the same (integer)
value as for workers can give very strange results.

Further comments are still welcome... :)

/siri







2014-07-01 14:43 GMT+02:00 Daniel Luna <daniel@REDACTED>:

> I have to mention that I think brutal_kill is a really bad choice for a
> default.
>
> It breaks the expectation in Erlang that supervisors can never crash. An
> expectation that is in the core of what makes Erlang the great language
> that it is.
>
> Dangling process trees are not fun to deal with (and don't happen on a VM
> crash).
>
> To preserve the correctness and trust of supervision trees the default
> should be either infinity or depend on the child type.
>
> For worker children brutal_kill is fine. For supervisor children infinity
> is the only sensible choice.
> On Jul 1, 2014 7:06 AM, "Loïc Hoguin" <essen@REDACTED> wrote:
>
>> Just wanted to say I agree with everything José said, except 3. the
>> different defaults for different process types. It makes things too hard to
>> keep track of.
>>
>> I think brutal_kill is a great choice because it makes it easier for the
>> user to stumble upon cold-start issues when restarting their
>> application/release. The VM may crash so there's never a guarantee to run
>> shutdown functions anyway, and too few people forget to test cases when
>> these didn't run, leading to potential production disasters.
>>
>> On 06/30/2014 04:37 PM, José Valim wrote:
>>
>>> I have some feedback based on my experience in writing and teaching
>>> Erlang/Elixir.
>>>
>>> 1. I would always require the supervision strategy. When I am
>>> prototyping, it is usually hard for me to reason about the intensity and
>>> restarts, but I can always reason about the supervision strategy. All
>>> teaching materials I know of also discuss the supervision right away, so
>>> having it explicit shouldn't hurt those cases.
>>>
>>> For such cases, maybe we should use the format of {{strategy(), map()},
>>> [children()]} (which is similar to what we have today).
>>>
>>> 2. I also agree with Richard. Having a default of 0 restarts may cause
>>> confusion and forcing people to define them won't help when they still
>>> have no idea of what values they should use (because it is prototyping /
>>> early deploys). I usually fine tune those values based on the production
>>> system reports so having saner defaults, like Ulf's, may make more sense.
>>>
>>> 3. The child_spec defaults are great and similar to the ones we use in
>>> Elixir with the only difference being the shutdown value. We have a
>>> default of 5000, which is the one usually recommended, but we set it to
>>> "infinity" if the type is a "supervisor" (which is the OTP docs
>>> recommendation if I remember correctly).
>>>
>>> Regarding 3., if the recommendation and the consensus in actual systems
>>> is to use brutal_kill for shutdown, I will gladly change Elixir to make
>>> it consistent with OTP defaults.
>>>
>>> Thanks for opening the changes for discussion!
>>>
>>> *José Valim*
>>> www.plataformatec.com.br <http://www.plataformatec.com.br/>
>>> Skype: jv.ptec
>>> Founder and Lead Developer
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> erlang-patches mailing list
>>> erlang-patches@REDACTED
>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>
>>>
>> --
>> Loïc Hoguin
>> http://ninenines.eu
>> _______________________________________________
>> erlang-patches mailing list
>> erlang-patches@REDACTED
>> http://erlang.org/mailman/listinfo/erlang-patches
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://erlang.org/pipermail/erlang-patches/attachments/20140703/3441c681/attachment.htm>