----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41285/#review115311 -----------------------------------------------------------
Patch looks great! Reviews applied: [41285] Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh - Mesos ReviewBot On Jan. 20, 2016, 1:41 a.m., Anand Mazumdar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41285/ > ----------------------------------------------------------- > > (Updated Jan. 20, 2016, 1:41 a.m.) > > > Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone. > > > Bugs: MESOS-3550 > https://issues.apache.org/jira/browse/MESOS-3550 > > > Repository: mesos > > > Description > ------- > > Currently, most of our logic for finding the executor type is based on the > fields pid/http in the Executor struct. We were erroneously setting the > initial pid value to be UPID() instead of being None(). > > The value of pid being UPID() is only possible in this scenario: > > We were unable to find the type of executor upon agent recovery i.e. no > pid/http marker file was found. We then mark this special case as pid=UPID(). > > This special case helps us while recovery in the following ways: > > - If the agent crashed before checkpointing the pid file, both executor->pid > and executor->http would be None(). This is similar to the case for HTTP > based executors (pid/http being None). In order, to distinguish between these > two cases, we set the pid=UPID(). This helps us in not shutting the HTTP > executor accidently by destroying the container when the agent is recovered > using recovery=cleanup > - This special value also helps us to do better logging by correctly > distinguishing between HTTP based executors and agent dying before > checkpointing the pid/http marker file. ( Both have pid/http as None) > > > Diffs > ----- > > src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a > > Diff: https://reviews.apache.org/r/41285/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Anand Mazumdar > >
