> On June 5, 2014, 1:38 p.m., Adam B wrote:
> > src/master/http.cpp, line 549
> > <https://reviews.apache.org/r/22223/diff/5/?file=604215#file604215line549>
> >
> >     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.
> 
> Yifan Gu wrote:
>     I have tried that, but that will cause a compiling error
>     
>     "error: macro "foreachvalue" passed 3 arguments, but takes just 2
>            foreachvalue (const hashmap<TaskID, Task*>& tasks, slave->tasks) {"
>     
>     I also have tried 
>     foreachvalue (hashmap<TaskID, Task*>& tasks, slave->tasks)
>     foreachvalue (hashmap<TaskID, Task*> tasks, slave->tasks)
>     
>     Same error, very strange.
>

Interesting. Guess the macro isn't smart enough to parse that correctly. Feel 
free to drop the issue and leave as is. C++11 'auto' might make this easier.


> On June 5, 2014, 1:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, lines 1992-1993
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1992>
> >
> >     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?
> 
> Yifan Gu wrote:
>     Haha, that's what I found yesterday. 
>     The newly selected master have no knowledge about this framework, so even 
> the framework tries to re-register with the master, the master will reply a 
> FrameworkRegisteredMessage.
>     
>     And this FrameworkRegisteredMessage is reply to the second trial of the 
> framework (because I dropped the first RegisterFrameworkMessage).
>     So if I put it before the clock::Advance(), then it can't get the message 
> since the clock is paused, it never retry doReliableRegistration().
>     
>

Makes sense. Carry on. You can drop this issue too.


- Adam


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


On June 5, 2014, 3:40 p.m., Yifan Gu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22223/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 3:40 p.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.
> 3, Havent' do the WebUI js part.
> 
> 
> Thanks,
> 
> Yifan Gu
> 
>

Reply via email to