> On Feb. 4, 2015, 11:55 p.m., Niklas Nielsen wrote: > > src/slave/flags.hpp, lines 133-136 > > <https://reviews.apache.org/r/30580/diff/1/?file=846974#file846974line133> > > > > Could we include that this is the inner most, first and/or the > > effective grace period (taken that we have a hierarchy of graceful > > shutdowns which need this period + delta to allow enough time for each > > step)? > > Ben Mahler wrote: > Hm, this mentions that this the command executor timeout and the > driver/slave use slightly larger timeouts. Do you have a concrete suggestion > of how to rephrase this?
I think this naming is the most unintuitive part for me. We say it is EXECUTOR_SHUTDOWN_GRACE_PERIOD but it is actually only directly relevant to the command executor. Why not have this repesent the slave's timeout as it was originally intended and subtract deltas for driver and command executor. We can require this to be atleast 3s (as alluded to in the flags comment) by having a check in Slave::initialize(). - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30580/#review71060 ----------------------------------------------------------- On Feb. 4, 2015, 2:14 a.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30580/ > ----------------------------------------------------------- > > (Updated Feb. 4, 2015, 2:14 a.m.) > > > Review request for mesos, Alexander Rukletsov, Niklas Nielsen, and Vinod Kone. > > > Repository: mesos > > > Description > ------- > > This is a pure cleanup to clarify the documentation and naming around > graceful shutdown. > > Note that I also namespaced the methods differently. > > > Diffs > ----- > > src/exec/exec.cpp aada24664dba9060a92230e25689c89852585443 > src/launcher/executor.cpp 1cf28f168cac6e8c7e98686a35509c2b4e052504 > src/slave/constants.hpp 761cfafb3b1b342af4d1dbdb2dec39a45dd62794 > src/slave/flags.hpp 0f6cc41d60a2e3bc2121cc438351135541ef99ba > src/slave/graceful_shutdown.hpp 59f5cfba032a81b5f69c2dd1bc1d96527686127c > src/slave/graceful_shutdown.cpp 04d8f091dfc5a007ac46ccc91c7ff7bfe620b524 > src/slave/slave.cpp a8b262174ab5c9a524db8318d3d1438cd75a702b > src/tests/gc_tests.cpp 454f0974833ad5db8b504a36b010cc72c3a19751 > src/tests/slave_tests.cpp e7e2af63da785644f3f7e6e23607c02be962a2c6 > > Diff: https://reviews.apache.org/r/30580/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
