Re: allow use of proplists in supervisor start specs
Siri Hansen <[email protected]> Fri, 4 Jul 2014 12:13:59 +0200
| Newsgroups | gmane.comp.lang.erlang.patches |
|---|---|
| Message-ID | <CAGqERUGWZ789SuwDC6g3ouc1kAFVzjNOeU_j67vu+=S_yFddLQ@mail.gmail.com> |
I have pushed the current status of this job to github. Comments still welcome! /siri https://github.com/sirihansen/otp/compare/erlang:980d7fa32128d43d36fb9527df1967eff801be9c...siri/sup-spec-maps/OTP-11043 2014-07-03 20:24 GMT+02:00 DeadZen <[email protected]>: > +1 on the latest revision > > On Thu, Jul 3, 2014 at 9:57 AM, Siri Hansen <[email protected]> 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 <[email protected]>: > > > >> 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" <[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 > > > _______________________________________________ erlang-patches mailing list [email protected] http://erlang.org/mailman/listinfo/erlang-patches