> On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.hpp, line 391 > > <https://reviews.apache.org/r/10142/diff/7/?file=280869#file280869line391> > > > > If present tense above, then present tense here as well.
switched to present tense. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1139 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1139> > > > > Why? If the parent dies and the child registers, we end up in this situation. Beefed up the comment, here and in reregisterExecutor(). > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1261 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1261> > > > > Signal to whom? killed the signaling bit. we no longer use the pid to signify registration. we use the executor state instead. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1375 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1375> > > > > newline? This code changed in a subsequent review, so this is N/A. I will avoid touching it here, to make the rebase easy. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1382 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1382> > > > > Can you either change this to an else if RUNNING or do a CHECK(RUNNING) > > on the framework state? I will fix this in the downstream review that changes this logic. Thanks for catching. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1384 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1384> > > > > newline? see above. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1393 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1393> > > > > Ditto for the executor state here. Either if or CHECK for RUNNING, or > > all the expected states. see above. > On April 12, 2013, 11:51 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, line 1824 > > <https://reviews.apache.org/r/10142/diff/7/?file=280870#file280870line1824> > > > > Do you want to CHECK the expected states? Again. Dropping this in favor of fixing it in the review that refactors this code. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10142/#review19115 ----------------------------------------------------------- On April 12, 2013, 9:24 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10142/ > ----------------------------------------------------------- > > (Updated April 12, 2013, 9:24 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > This is based off https://reviews.apache.org/r/10112. > > Also fixed TODOs and other misc stuff from the above review. > > > Diffs > ----- > > src/slave/isolator.hpp d702041784f5db159efd7da4d916405e86d99741 > src/slave/slave.hpp 2529bf500a3265b10ad4cddde10c2d62a6cdb4a0 > src/slave/slave.cpp 325231458a6883019436e7cc5a37f85f0f5735fa > src/slave/status_update_manager.hpp > e6ca40c5c05c0952cf76fb1db7eff2e4270c0d24 > src/slave/status_update_manager.cpp > 044d245f370ef23ddc67fadbf7f8fe9d75dd662a > src/tests/isolator.hpp f885ccb44e809383e658f45d9a03eda174cf2d72 > src/tests/slave_recovery_tests.cpp d0ff9b73e06e89a5409f038be2766333e0a0689e > > Diff: https://reviews.apache.org/r/10142/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
