Re: allow use of proplists in supervisor start specs

To clarify my opening point, I do not like the idea of brutally killing
supervisors. I have nothing against brutally killing children of type
worker.


On 1 July 2014 13:41, James Fish <[email protected]> wrote:

> I think brutal_kill is a bad choice for supervisor children. When a
> supervisor process exits it is very beneficial to know that (except for
> very exceptional circumstances) every process below it in the supervisor
> tree has exited. If these processes lower down the supervision have not
> shutdown, they are now orphaned with no guarantee they will be
> shutdown/exit.
>
> The issue materialises when the child supervisor has a worker that traps
> exits. The worker might be slow to terminate, perhaps because it has alot
> of messages queued, makes a long blocking call or has got stuck in a call
> that will never return due to a bug. When properly terminated by it's
> parent, the worker might be killed or given a shutdown timeout but this
> does not come into effect because its parent was killed, so the worker
> remains alive for an undetermined period of time.
>
> If/When the worker's parent is restarted a new version of the worker can
> not be (re)started if the worker is registered because the old worker is
> still alive. This could cause the supervisor that brutal kills it's child
> supervisor to max out it's restart limit and continue to do so up the
> supervision tree. This could have been avoided if the worker's parent had
> been allowed to carry out its duty as a supervisor to terminate its
> children.
>
> Of course if the worker is not registered a new version can be restarted.
> However in this case there could now be two versions of this worker when
> the application was only designed to have one running at a time. This bug
> probably never crops up in testing, perhaps because it was assumed the
> worker's supervisor would prevent this case.
>
> Note that everywhere above where I say the child is a worker, I could have
> used the child type supervisor! This problem could go all the way down the
> supervision tree.
>
> I do not follow the view that different defaults for different types is
> too complicated, it is one of these least complicated configuration options
> in a supervisor spec and there are only two types.
>
>
> On 1 July 2014 12:06, Loïc Hoguin <[email protected]> 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
>>> [email protected]
>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>
>>>
>> --
>> Loïc Hoguin
>> http://ninenines.eu
>>
>> _______________________________________________
>> erlang-patches mailing list
>> [email protected]
>> http://erlang.org/mailman/listinfo/erlang-patches
>>
>
>

_______________________________________________
erlang-patches mailing list
[email protected]
http://erlang.org/mailman/listinfo/erlang-patches