> On June 5, 2014, 8: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.
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.
> On June 5, 2014, 8:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, line 1925
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1925>
> >
> > s/will be appear/will appear/
Will do.
> On June 5, 2014, 8:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, lines 1943-1944
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1943>
> >
> > If you just call driver(&sched, &detector); it will fill in the default
> > FwkInfo and credential.
Will do.
> On June 5, 2014, 8:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, line 1946
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1946>
> >
> > resourceOffers(&driver, _)
Will do.
> On June 5, 2014, 8:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, line 1954
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1954>
> >
> > statusUpdate(&driver, _)
Will do.
> On June 5, 2014, 8: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?
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().
> On June 5, 2014, 8:38 p.m., Adam B wrote:
> > src/tests/master_tests.cpp, lines 1976-1979
> > <https://reviews.apache.org/r/22223/diff/5/?file=604216#file604216line1976>
> >
> > 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?
Good idea, will do! Thanks!
- Yifan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22223/#review44844
-----------------------------------------------------------
On June 5, 2014, 6:47 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, 6:47 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.
>
>
> Thanks,
>
> Yifan Gu
>
>