Re: Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Vinod Kone]

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

Ship it!



src/tests/reconciliation_tests.cpp


s/we do this// ?


- Vinod Kone


On Sept. 18, 2014, 9:30 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25568/
> ---
> 
> (Updated Sept. 18, 2014, 9:30 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-1410
> https://issues.apache.org/jira/browse/MESOS-1410
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added two tests:
> 
> (1) Ensure that reconciliation works for terminal unacknowledged tasks.
> (2) Ensure that resources are released for terminal unacknowledged tasks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 52a7409f7132f89f4a64e589fa096dd07f527fd9 
>   src/tests/master_tests.cpp ff2b50f517d7c413419e6c7bc823cf5e2cdff1aa 
>   src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 
> 
> Diff: https://reviews.apache.org/r/25568/diff/
> 
> 
> Testing
> ---
> 
> make check, ran these new tests in repetition
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Mesos ReviewBot]

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


Bad patch!

Reviews applied: [25565]

Failed command: git apply --index 25565.patch

Error:
 error: patch failed: src/master/master.hpp:368
error: src/master/master.hpp: patch does not apply
error: patch failed: src/master/master.cpp:649
error: src/master/master.cpp: patch does not apply

- Mesos ReviewBot


On Sept. 18, 2014, 9:30 p.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25568/
> ---
> 
> (Updated Sept. 18, 2014, 9:30 p.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-1410
> https://issues.apache.org/jira/browse/MESOS-1410
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added two tests:
> 
> (1) Ensure that reconciliation works for terminal unacknowledged tasks.
> (2) Ensure that resources are released for terminal unacknowledged tasks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp 52a7409f7132f89f4a64e589fa096dd07f527fd9 
>   src/tests/master_tests.cpp ff2b50f517d7c413419e6c7bc823cf5e2cdff1aa 
>   src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 
> 
> Diff: https://reviews.apache.org/r/25568/diff/
> 
> 
> Testing
> ---
> 
> make check, ran these new tests in repetition
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Ben Mahler]

> On Sept. 16, 2014, 6:44 a.m., Vinod Kone wrote:
> > src/tests/master_tests.cpp, line 2329
> > 
> >
> > What is the guarantee that offers2 will be made after task's resources 
> > are recovered?

I forgot to pass the resource flags for the slave in per your comment above. 
The gaurantee is now that we use the full offer, so no resources remain.

If we ever break that guarantee, we'll catch it here and the test would fail:

```
  // Ensure we get all of the resources back.
  EXPECT_EQ(offers1.get()[0].resources(), offers2.get()[0].resources());
```

I also added a Clock::advance to avoid waiting around for the allocation 
interval.


> On Sept. 16, 2014, 6:44 a.m., Vinod Kone wrote:
> > src/tests/reconciliation_tests.cpp, lines 734-735
> > 
> >
> > Why do you want to make sure that master received the reconcile tasks 
> > message? Isn't the receipt of the update guarantee that?
> > 
> > Actually, thinking a bit more, how do you guarantee that the second 
> > update was due to reconcile tasks and not a retry by the slave?

Removed this, was an artifact of needing Clock::settle from other tests.

I've now made it so that only the first StatusUpdateMessage from slave to 
master is delivered. All other StatusUpdateMessages will be dropped (confirmed 
this worked by temporarily adding a Clock::advance() loop to force the slave to 
retry).


- Ben


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


On Sept. 12, 2014, 2:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25568/
> ---
> 
> (Updated Sept. 12, 2014, 2:01 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-1410
> https://issues.apache.org/jira/browse/MESOS-1410
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added two tests:
> 
> (1) Ensure that reconciliation works for terminal unacknowledged tasks.
> (2) Ensure that resources are released for terminal unacknowledged tasks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d5db24ef3c2d2501aa5852b62d50a425bc0ad925 
>   src/tests/master_tests.cpp 3d080b2efad5a210353d4cef4c827380d5138d1a 
>   src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 
> 
> Diff: https://reviews.apache.org/r/25568/diff/
> 
> 
> Testing
> ---
> 
> make check, ran these new tests in repetition
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Re: Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Ben Mahler]

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

(Updated Sept. 18, 2014, 9:30 p.m.)


Review request for mesos, Niklas Nielsen and Vinod Kone.


Changes
---

Improved the tests per Vinod's review.


Bugs: MESOS-1410
https://issues.apache.org/jira/browse/MESOS-1410


Repository: mesos-git


Description
---

Added two tests:

(1) Ensure that reconciliation works for terminal unacknowledged tasks.
(2) Ensure that resources are released for terminal unacknowledged tasks.


Diffs (updated)
-

  src/master/master.cpp 52a7409f7132f89f4a64e589fa096dd07f527fd9 
  src/tests/master_tests.cpp ff2b50f517d7c413419e6c7bc823cf5e2cdff1aa 
  src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 

Diff: https://reviews.apache.org/r/25568/diff/


Testing
---

make check, ran these new tests in repetition


Thanks,

Ben Mahler

Re: Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Vinod Kone]

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



src/tests/master_tests.cpp


Hmm... This is not passed to StartSlave()?



src/tests/master_tests.cpp


s/task terminal/task in terminal state/



src/tests/master_tests.cpp


The "Ignore subsequent offers" comment is incorrect here.



src/tests/master_tests.cpp


What is the guarantee that offers2 will be made after task's resources are 
recovered?



src/tests/reconciliation_tests.cpp


s/ensure/ensures/



src/tests/reconciliation_tests.cpp


s/task terminal/task in terminal state/



src/tests/reconciliation_tests.cpp


Why do you want to make sure that master received the reconcile tasks 
message? Isn't the receipt of the update guarantee that?

Actually, thinking a bit more, how do you guarantee that the second update 
was due to reconcile tasks and not a retry by the slave?


- Vinod Kone


On Sept. 12, 2014, 2:01 a.m., Ben Mahler wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25568/
> ---
> 
> (Updated Sept. 12, 2014, 2:01 a.m.)
> 
> 
> Review request for mesos, Niklas Nielsen and Vinod Kone.
> 
> 
> Bugs: MESOS-1410
> https://issues.apache.org/jira/browse/MESOS-1410
> 
> 
> Repository: mesos-git
> 
> 
> Description
> ---
> 
> Added two tests:
> 
> (1) Ensure that reconciliation works for terminal unacknowledged tasks.
> (2) Ensure that resources are released for terminal unacknowledged tasks.
> 
> 
> Diffs
> -
> 
>   src/master/master.cpp d5db24ef3c2d2501aa5852b62d50a425bc0ad925 
>   src/tests/master_tests.cpp 3d080b2efad5a210353d4cef4c827380d5138d1a 
>   src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 
> 
> Diff: https://reviews.apache.org/r/25568/diff/
> 
> 
> Testing
> ---
> 
> make check, ran these new tests in repetition
> 
> 
> Thanks,
> 
> Ben Mahler
> 
>

Review Request 25568: Added tests for terminal unacknowledged tasks in the Master.

[Ben Mahler]

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

Review request for mesos, Niklas Nielsen and Vinod Kone.


Bugs: MESOS-1410
https://issues.apache.org/jira/browse/MESOS-1410


Repository: mesos-git


Description
---

Added two tests:

(1) Ensure that reconciliation works for terminal unacknowledged tasks.
(2) Ensure that resources are released for terminal unacknowledged tasks.


Diffs
-

  src/master/master.cpp d5db24ef3c2d2501aa5852b62d50a425bc0ad925 
  src/tests/master_tests.cpp 3d080b2efad5a210353d4cef4c827380d5138d1a 
  src/tests/reconciliation_tests.cpp 1c9e73b0ee99a8a33f663f992b0c9770e83b98c5 

Diff: https://reviews.apache.org/r/25568/diff/


Testing
---

make check, ran these new tests in repetition


Thanks,

Ben Mahler