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

[DeadZen]

+1 on the latest revision

On Thu, Jul 3, 2014 at 9:57 AM, Siri Hansen <erlangsiri@REDACTED> wrote:
> 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
>
>
>
> _______________________________________________
> erlang-patches mailing list
> erlang-patches@REDACTED
> http://erlang.org/mailman/listinfo/erlang-patches
>