-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/16724/#review32949
-----------------------------------------------------------

Ship it!



src/master/master.cpp
<https://reviews.apache.org/r/16724/#comment62002>

    Let's use the same parameter name here and in the declaration please (same 
for Master::readdCompletedFramework too).



src/master/master.cpp
<https://reviews.apache.org/r/16724/#comment61999>

    We align our wrapped "lists" (parameter lists, argument lists, etc).



src/master/master.cpp
<https://reviews.apache.org/r/16724/#comment62004>

    = None();



src/master/master.cpp
<https://reviews.apache.org/r/16724/#comment62003>

    s/knownFramework/completedFramework/



src/master/master.cpp
<https://reviews.apache.org/r/16724/#comment62005>

    Let's make this a TODO to make registration and reregistration time be 
optional in Framework.



src/messages/messages.proto
<https://reviews.apache.org/r/16724/#comment62008>

    I was thinking we'd put this in the 'archive' package since we'll likely 
call a class 'Archive', but this is fine for now and we can always rename it 
later without causing upgrade/compatibility issues.



src/messages/messages.proto
<https://reviews.apache.org/r/16724/#comment62011>

    Can we make this optional? I can see moving something like 'pid' into 
another structure and I don't think it's that big of a deal if it's not present 
(it's only used for constructing a completed 'Framework' and not having the pid 
is not a big deal.



src/slave/slave.cpp
<https://reviews.apache.org/r/16724/#comment62014>

    Please kill all the 'msg' prefixes here, they're not necessary and not with 
convention.



src/slave/slave.cpp
<https://reviews.apache.org/r/16724/#comment62013>

    Please align the wrapping here.


- Benjamin Hindman


On Jan. 17, 2014, 12:24 a.m., Adam B wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/16724/
> -----------------------------------------------------------
> 
> (Updated Jan. 17, 2014, 12:24 a.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Ben Mahler, Niklas Nielsen, and 
> Vinod Kone.
> 
> 
> Bugs: MESOS-767
>     https://issues.apache.org/jira/browse/MESOS-767
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added completed frameworks/tasks to slave re-registration.
> Fixes MESOS-767.
> 
> Additional issues discovered during investigation:
> - MESOS-905: Remove Framework.id in favor of FrameworkInfo.id
> - MESOS-906: Last task in Completed Framework never graduates from
> terminatedTasks to completedTasks.
> - Completed frameworks/executors/tasks are stored in circular buffers,
> and these may overflow in different orders on different slaves. 
> BenH proposes an archive to replace these circular buffers.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 95b9cec 
>   src/master/master.cpp 38c5532 
>   src/messages/messages.proto 1f264d5 
>   src/slave/slave.cpp 396293b 
> 
> Diff: https://reviews.apache.org/r/16724/diff/
> 
> 
> Testing
> -------
> 
> make check; manually failed-over a master, watched the slave reregister its 
> completed frameworks, web UI shows completed tasks and stdout/stderr.
> 
> 
> Thanks,
> 
> Adam B
> 
>

Reply via email to