> On Aug. 3, 2016, 6:12 p.m., Neil Conway wrote:
> > src/tests/master_tests.cpp, line 4665
> > <https://reviews.apache.org/r/50723/diff/1/?file=1460861#file1460861line4665>
> >
> > Should we also check that the task correctly transitions to a terminal
> > state in the master?
>
> Anand Mazumdar wrote:
> I had punted on this for now and just ensuring if the resources have been
> correctly recovered. I left a TODO to address this for later.
>
> Neil Conway wrote:
> The observed behavior in the bug report is about task statuses, not
> resource allocation; those are fairly different things. I'd personally prefer
> to see a test for task statuses be included as part of this change.
hmm, They might be fairly different things from the view point of the user but
are closely aligned from the perspective of our code.
1. This is how the general flow of the code is in `updateTask()`:
```cpp
task.set_state(state);
if (terminated(state)) {
allocator->recoverResources(...);
}
```
Hence, what the test currently does is a _strict_ superset by checking for
recovered resources. Of course, the end user who is not familiar with the
internals would only look for what they see on the UI via the `/state` call!
2. The only way I could think of to ensure that the task was _updated_ was to
make another call to `/state` endpoint. That seemed un-necessary due to 1. and
too verbose and hence I left a TODO.
But then, I thought that I would just go ahead and address the `TODO` now and
found another bug in the `GET_TASKS` implementation in the master. CR:
https://reviews.apache.org/r/50815 :(
- Anand
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/50723/#review144633
-----------------------------------------------------------
On Aug. 3, 2016, 11:14 p.m., Anand Mazumdar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50723/
> -----------------------------------------------------------
>
> (Updated Aug. 3, 2016, 11:14 p.m.)
>
>
> Review request for mesos, Adam B, Neil Conway, and Vinod Kone.
>
>
> Bugs: MESOS-5930
> https://issues.apache.org/jira/browse/MESOS-5930
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The master's status handler function used to ignore the status updates
> from the agents for frameworks not yet re-connected with the master
> upon a failover. This change modifies that logic to still update
> the local state and not bail out early.
>
>
> Diffs
> -----
>
> src/master/master.cpp 17d5c5875647bb35e2518cc2bd9e134b020c05bf
> src/tests/master_tests.cpp 6709818d599c068c289bcb714446018577082d8b
>
> Diff: https://reviews.apache.org/r/50723/diff/
>
>
> Testing
> -------
>
> make check (gtest_repeat=100)
>
>
> Thanks,
>
> Anand Mazumdar
>
>
