-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60104/#review178320
-----------------------------------------------------------
src/slave/slave.hpp
Lines 337 (patched)
<https://reviews.apache.org/r/60104/#comment252204>
What is `later` referring to here? Remove it?
src/slave/state.cpp
Lines 82 (patched)
<https://reviews.apache.org/r/60104/#comment252244>
Why initialize it here? Initialize it inside the class definition?
src/slave/state.cpp
Line 83 (original), 84 (patched)
<https://reviews.apache.org/r/60104/#comment252206>
Is this question here necessary? :)
src/slave/state.cpp
Lines 85-87 (patched)
<https://reviews.apache.org/r/60104/#comment252208>
"restartable tasks" isn't the sole reason for keeping the agent ID. (you could lose all
tasks but still wanted to keep the ID).
I think these details are not important for now and if we never had short-circuited agent
recovery in the reboot case and we are implementing agent recovery from scratch now, recovering
the agent regardless of reboot is actually pretty straighforward.
Hence I actually don't feel there's anything perticular that's worth commenting here.
We could document the behavior change in the commit message / review summary.
We should document the code as if all patches in review have already landed and document
the commits with more context about what changed and why.
- Jiang Yan Xu
On June 15, 2017, 12:31 p.m., Megha Sharma wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60104/
> -----------------------------------------------------------
>
> (Updated June 15, 2017, 12:31 p.m.)
>
>
> Review request for mesos, Neil Conway, Vinod Kone, and Jiang Yan Xu.
>
>
> Bugs: MESOS-6223
> https://issues.apache.org/jira/browse/MESOS-6223
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Added rebooted flag to State and Slave.RecoveryInfo to remember
> if the agent has rebooted.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 7ffaed14035a05259ec72c70532ee4f0affa1f5d
> src/slave/slave.cpp 7d147ac6609933ac884bfc29032dba572a0952c6
> src/slave/state.hpp a497ce1f58fb8dc7718ee5bb10bc62dd7479efa5
> src/slave/state.cpp 18b790d2cc4f537cc9b0c3cca59b9cbaac0eda10
>
>
> Diff: https://reviews.apache.org/r/60104/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Megha Sharma
>
>
|