> On Sept. 6, 2016, 7:33 p.m., Vinod Kone wrote: > > > > Vinod Kone wrote: > also, can you add a test for this?
Sure, I can add a test. I am just wondering how to use a test to verify the logic we added in this patch, maybe we can just add a test to launch a short task and check `ContainerTermination.status` is 0 when `Slave::executorTerminated()` is call which means the HTTP command executor has received the ACK from agent for the terminal status update. Any comments? > On Sept. 6, 2016, 7:33 p.m., Vinod Kone wrote: > > src/launcher/executor.cpp, lines 521-522 > > <https://reviews.apache.org/r/46187/diff/5/?file=1488422#file1488422line521> > > > > why this change? Sorry, it was introduced by mistake, will remove it. > On Sept. 6, 2016, 7:33 p.m., Vinod Kone wrote: > > src/launcher/executor.cpp, line 682 > > <https://reviews.apache.org/r/46187/diff/5/?file=1488422#file1488422line682> > > > > hmm. what's the guarantee that an HTTP based executor receives an ACK > > within a second? what if the agent is down? If executor does not receives an ACK from agent within 1 second, that means there should be something wrong in agent, so with this code, the executor will exit with -1 and a message so that we can catch such situation. Maybe we should enlarge this timeout a bit (e.g., 3 seconds) to be safer? - Qian ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46187/#review147808 ----------------------------------------------------------- On Aug. 30, 2016, 2:01 p.m., Qian Zhang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46187/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2016, 2:01 p.m.) > > > Review request for mesos, Anand Mazumdar and Vinod Kone. > > > Bugs: MESOS-5276 > https://issues.apache.org/jira/browse/MESOS-5276 > > > Repository: mesos > > > Description > ------- > > Terminate when receiving the ACK of terminal status update. > > > Diffs > ----- > > src/launcher/executor.cpp 5370634ef9e6f3ac9717fed71f6a77707026a16a > > Diff: https://reviews.apache.org/r/46187/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Qian Zhang > >
