> On Nov. 5, 2014, 7:50 p.m., Ben Mahler wrote: > > src/slave/slave.cpp, lines 1195-1200 > > <https://reviews.apache.org/r/27567/diff/1/?file=748326#file748326line1195> > > > > A comment here as to why we don't need to send TASK_LOST would be much > > appreciated! It's not obvious so someone might come along and add a > > TASK_LOST to make sure we're not dropping the task on the floor, so context > > here would be great! > > Bernd Mathiske wrote: > Hah, thanks for sharing - I am not alone! :-) None of this was obvious to > me either, because there is no comment explaining the general life cycle of > anything. Once you understand the intended life cycle, there is now way there > can be a TASK_LOST situation here, though. Therefore I propose adding > comments describing the overall picture regarding frameworks, executor IDs > and task creation in the appropriate places, instead. I'll file a ticket if > you agree.
> Once you understand the intended life cycle, there is now way there can be a > TASK_LOST situation here, though. Phew! :) Could you distill your learnings into a comment here, and maybe make the log message more informative? Even with an overall description as you mentioned, dummies like me would still get confused here given the lack of _local_ context. ;) - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27567/#review60016 ----------------------------------------------------------- On Nov. 4, 2014, 11:46 p.m., Bernd Mathiske wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27567/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2014, 11:46 p.m.) > > > Review request for mesos and Vinod Kone. > > > Bugs: MESOS-2038 > https://issues.apache.org/jira/browse/MESOS-2038 > > > Repository: mesos-git > > > Description > ------- > > Removed a few lines of dead code that coverty discovered. > > > Diffs > ----- > > src/slave/slave.cpp 5e9b0e4f93a5100a91340e1f6fb1fe160b2eea4b > > Diff: https://reviews.apache.org/r/27567/diff/ > > > Testing > ------- > > none. > expecting/waiting for review bot to report no problem. > > > Thanks, > > Bernd Mathiske > >
