Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 30, 2017, 5 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp 823bd440f5b25ad363909775130dd9bf34d43a7b 


Diff: https://reviews.apache.org/r/63843/diff/8/

Changes: https://reviews.apache.org/r/63843/diff/7-8/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Jie Yu]

> On Nov. 29, 2017, 3:09 a.m., Jie Yu wrote:
> > src/tests/slave_tests.cpp
> > Lines 8900-8923 (patched)
> > 
> >
> > It'll be nice to move this logic here to 
> > MockResourceProvider::operationFailed and call `Invoke(...)` in the EXPECT 
> > above.

either drop or address?


- Jie


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


On Nov. 29, 2017, 4:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63843/
> ---
> 
> (Updated Nov. 29, 2017, 4:27 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whenever a speculated operation fails in a resource provider, we
> expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
> can rollback its agent state. This patch adds such a test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp aeaa923f36af134b61ad9df93c95651a8d9bc23c 
> 
> 
> Diff: https://reviews.apache.org/r/63843/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Jie Yu]

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


Ship it!




Ship It!

- Jie Yu


On Nov. 29, 2017, 4:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63843/
> ---
> 
> (Updated Nov. 29, 2017, 4:27 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whenever a speculated operation fails in a resource provider, we
> expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
> can rollback its agent state. This patch adds such a test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp aeaa923f36af134b61ad9df93c95651a8d9bc23c 
> 
> 
> Diff: https://reviews.apache.org/r/63843/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

> On Nov. 29, 2017, 4:09 a.m., Jie Yu wrote:
> > src/tests/slave_tests.cpp
> > Lines 8831-8839 (patched)
> > 
> >
> > Any reason these EXPECT are in the scope?

I put these here since the framework's subscription triggered them. I agree 
that they could be set just as well in the scope of the framework where I moved 
them now.


> On Nov. 29, 2017, 4:09 a.m., Jie Yu wrote:
> > src/tests/slave_tests.cpp
> > Lines 8933-8936 (patched)
> > 
> >
> > This is a bit subtle. If a speculative operation fails, the update 
> > state will inform the agent that the operation has failed. However, the 
> > agent will inform that master that the operation is still pending?
> > 
> > What's the downside of telling agent master the same thing (that the 
> > operation has failed)?
> > 
> > This probably needs more thinking.

This seems strongly related to 
https://reviews.apache.org/r/63731/#comment270131. Please see my comment there.


- Benjamin


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


On Nov. 29, 2017, 5:27 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63843/
> ---
> 
> (Updated Nov. 29, 2017, 5:27 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whenever a speculated operation fails in a resource provider, we
> expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
> can rollback its agent state. This patch adds such a test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp aeaa923f36af134b61ad9df93c95651a8d9bc23c 
> 
> 
> Diff: https://reviews.apache.org/r/63843/diff/7/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 29, 2017, 5:27 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Addressed comments from Jie.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp aeaa923f36af134b61ad9df93c95651a8d9bc23c 


Diff: https://reviews.apache.org/r/63843/diff/7/

Changes: https://reviews.apache.org/r/63843/diff/6-7/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Jie Yu]

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




src/tests/slave_tests.cpp
Lines 8831-8839 (patched)


Any reason these EXPECT are in the scope?



src/tests/slave_tests.cpp
Lines 8900-8923 (patched)


It'll be nice to move this logic here to 
MockResourceProvider::operationFailed and call `Invoke(...)` in the EXPECT 
above.



src/tests/slave_tests.cpp
Lines 8933-8936 (patched)


This is a bit subtle. If a speculative operation fails, the update state 
will inform the agent that the operation has failed. However, the agent will 
inform that master that the operation is still pending?

What's the downside of telling agent master the same thing (that the 
operation has failed)?

This probably needs more thinking.


- Jie Yu


On Nov. 28, 2017, 9:32 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63843/
> ---
> 
> (Updated Nov. 28, 2017, 9:32 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Whenever a speculated operation fails in a resource provider, we
> expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
> can rollback its agent state. This patch adds such a test.
> 
> 
> Diffs
> -
> 
>   src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 
> 
> 
> Diff: https://reviews.apache.org/r/63843/diff/6/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 28, 2017, 10:32 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Renamed test.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 


Diff: https://reviews.apache.org/r/63843/diff/6/

Changes: https://reviews.apache.org/r/63843/diff/5-6/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 27, 2017, 3:55 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Simplified test by using default actions of `MockResourceProvider`.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 


Diff: https://reviews.apache.org/r/63843/diff/5/

Changes: https://reviews.apache.org/r/63843/diff/4-5/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 24, 2017, 3:07 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp a2274b691cf94b003c4bc15450d176a9c73517d5 


Diff: https://reviews.apache.org/r/63843/diff/4/

Changes: https://reviews.apache.org/r/63843/diff/3-4/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 21, 2017, 10:09 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Rebased.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp f5befc204d1b6c5af86195980004a8486449bd38 


Diff: https://reviews.apache.org/r/63843/diff/3/

Changes: https://reviews.apache.org/r/63843/diff/2-3/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

(Updated Nov. 17, 2017, 6:24 p.m.)


Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Changes
---

Extended test to check that resources from failed operations are offered again 
unmodified.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs (updated)
-

  src/tests/slave_tests.cpp a75bb260df223b5b86f31e91eec1b6ba8db00cb2 


Diff: https://reviews.apache.org/r/63843/diff/2/

Changes: https://reviews.apache.org/r/63843/diff/1-2/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Review Request 63843: Implemented a test of offer operation reconcilation.

[Benjamin Bannier]

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

Review request for mesos, Greg Mann, Jie Yu, and Jan Schlicht.


Repository: mesos


Description
---

Whenever a speculated operation fails in a resource provider, we
expect the agent to trigger a 'UpdateSlaveMessage' to the master so it
can rollback its agent state. This patch adds such a test.


Diffs
-

  src/tests/slave_tests.cpp 61dd940e4452a1dbc6c0361bfc917bdc2dd16be8 


Diff: https://reviews.apache.org/r/63843/diff/1/


Testing
---

`make check`


Thanks,

Benjamin Bannier