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


Looks great!
Have you tested this manually to see the tasks/framework show up in the web ui? 
Mention that in the Testing section, please.


src/master/http.cpp
<https://reviews.apache.org/r/22223/#comment79402>

    I agree with Niklas that it doesn't add much to define a typedef here 
that's only used once. Go ahead and inline it like:
    foreachvalue (const hashmap<TaskID, Task*>& tasks, slave->tasks) {
    Looks like it would still fit in 80 chars anyway.



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79403>

    s/will be appear/will appear/



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79405>

    If you just call driver(&sched, &detector); it will fill in the default 
FwkInfo and credential.



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79406>

    resourceOffers(&driver, _)



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79408>

    statusUpdate(&driver, _)



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79411>

    Do you want to do any additional validation that !frameworks.empty(), 
activeFramework's id is the expected default, and !activeTasks.empty() (only 
1?). Also verify no orphan_tasks or unknown_frameworks at this point?



src/tests/master_tests.cpp
<https://reviews.apache.org/r/22223/#comment79414>

    Perhaps this frameworkRegisteredMessage FUTURE_PROTOBUF should go down 
right before Clock::advance, to check that it doesn't happen until after the 
advance/settle/resume.
    Also, why isn't it FrameworkReregisteredMessage?


- Adam B


On June 5, 2014, 11:47 a.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22223/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 11:47 a.m.)
> 
> 
> Review request for mesos, Adam B, Niklas Nielsen, Ross Allen, and Vinod Kone.
> 
> 
> Bugs: MESOS-1312
>     https://issues.apache.org/jira/browse/MESOS-1312
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Added orphan_tasks and unknown_frameworks in state.json to display tasks that 
> are currently running on the slaves,
> but do not belong to any already (re-)registered frameworks.
> 
> 
> Diffs
> -----
> 
>   src/master/http.cpp b565dc6 
>   src/tests/master_tests.cpp 34df121 
> 
> Diff: https://reviews.apache.org/r/22223/diff/
> 
> 
> Testing
> -------
> 
> Added MasterTest.OrphanTasks to test:
> 1, Before the framework re-registers with the master:
>   Tasks that belongs to that frameworks will appear in the ["orphan_tasks"] 
> field.
>   That frameworks will appear in the ["not_yet_reregistered_frameworks"] 
> field.
> 2, After the framework re-registers with the master:
>   There will be no more orphan tasks and not-yet-reregistered frameworks.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>

Reply via email to