You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2016/01/20 02:28:52 UTC
Re: Review Request 41285: PID none bug in slave
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41285/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 1:28 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Explicit assign `pid` to `None()`
Summary (updated)
-----------------
PID none bug in slave
Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550
Repository: mesos
Description (updated)
-------
PID none bug in slave
Diffs (updated)
-----
src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a
Diff: https://reviews.apache.org/r/41285/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41285: Made Executor struct assign `pid/http` to be
None() explicitly.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
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
>
>
Re: Review Request 41285: Made Executor struct assign `pid/http` to be
None() explicitly.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
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.
Changes
-------
Initialized `http` to be None() too.
Summary (updated)
-----------------
Made Executor struct assign `pid/http` to be None() explicitly.
Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550
Repository: mesos
Description (updated)
-------
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 (updated)
-----
src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a
Diff: https://reviews.apache.org/r/41285/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar
Re: Review Request 41285: Made Executor struct assign `pid` to be
None() explicitly.
Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41285/
-----------------------------------------------------------
(Updated Jan. 20, 2016, 1:31 a.m.)
Review request for mesos, Ben Mahler, Jie Yu, and Vinod Kone.
Changes
-------
Updated summary and description.
Summary (updated)
-----------------
Made Executor struct assign `pid` to be None() explicitly.
Bugs: MESOS-3550
https://issues.apache.org/jira/browse/MESOS-3550
Repository: mesos
Description (updated)
-------
The executor assigned `pid` to `UPID()` instead of `None()`. The value `UPID()` is used as a special flag during recovery when the executor type is unknown i.e. no pid/http marker file is found.
Diffs
-----
src/slave/slave.cpp 759c8d5b1bfb5ad723aa423e1487998ed62bbc3a
Diff: https://reviews.apache.org/r/41285/diff/
Testing
-------
make check
Thanks,
Anand Mazumdar