Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Aug. 5, 2017, 4:40 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


Changes
---

Rebase recent changes.


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


Repository: mesos


Description (updated)
---

This implements some tests in previous todos for
quota functionality.

For posterity, removed comments are implemented in:
* An agent with quota'ed tasks disconnects and there are not enough free
  resources (alert and under quota situation): 
`InsufficientResourceAfterSlaveDisconnect`;
* An agent with quota'ed tasks disconnects and there are enough free
  resources (new offers): `SufficientAfterSlaveWithQuotaTaskDisconnect`
* Role quota is below its allocation (InverseOffer generation): This is not 
happening yet.
* Two roles, two frameworks, one is production but rejects offers, the
  other is greedy and tries to hijack the cluster which is prevented by
  quota: `GreedyFrameworkCannotHijackQuotaRole`
* Quota'ed and non-quota'ed roles, multiple frameworks in quota'ed role,
  ensure total allocation sums up to quota: `TotalAllocationSatisfiesQuota`
* Remove quota with no running tasks: `RemoveQuotaNoTask`
* Remove quota with running tasks: `RemoveQuotaWithTask`


Diffs (updated)
-

  src/tests/master_quota_tests.cpp 9d52f767c112abd55ab3f49d172eb6e3caea511b 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Feb. 10, 2017, 7:55 a.m.)


Review request for mesos, Alexander Rukletsov, Xiaojian Huang, and Neil Conway.


Changes
---

Rebase.


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


Repository: mesos


Description
---

This implements some tests in previous todos for
quota functionality.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp d15c7aacb85596cdee7cf59c0c179247ba624fe5 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Feb. 7, 2017, 7:13 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


Changes
---

Rebase and review comments.


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


Repository: mesos


Description (updated)
---

This implements some tests in previous todos for
quota functionality.


Diffs (updated)
-

  src/tests/master_quota_tests.cpp b25f5911579c435549b9bc65994627414357dcb6 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Alexander Rukletsov]

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




src/tests/master_quota_tests.cpp (lines 1721 - 1723)


We should probably rephrase this; see my comment below.



src/tests/master_quota_tests.cpp (lines 1837 - 1839)


Yeah, current behaviour indeed prevents satisfied quota'ed frameworks from 
receiving non-revocable resources beyond their quota. It probably makes sense 
to mention this assumption in the test comment.

You don't have to trigger an allocation though, because adding an agent 
triggers one. I think it's also fine to leave a note here that we expect no 
allocations here. We don't really have to check because if an offer arrives 
here the next expectation will fail. 

What I originially had in mind was some kind of test that quota is actually 
helpful when agents start dying in the cluster. We can simulate this situation 
as well, however without quota enforcement (read: preemption and revocation) 
this test does not seem to have a lot of sense. I suggest to punt on my 
original idea for now and just go with your proposal.



src/tests/master_quota_tests.cpp (lines 1843 - 1846)


Please keep formatting consistent throughout the file.



src/tests/master_quota_tests.cpp (lines 1862 - 1864)


The expectation should be set before the event is triggered, otherwise 
there is a race: the offer will be processed and ignored and hence 
`AWAIT_READY(offers2);` will never be triggered.


- Alexander Rukletsov


On Dec. 27, 2016, 11:24 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Dec. 27, 2016, 11:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-3982
> https://issues.apache.org/jira/browse/MESOS-3982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Alexander Rukletsov]

> On Nov. 12, 2016, 8:01 a.m., Zhitao Li wrote:
> > src/tests/master_quota_tests.cpp, line 1723
> > 
> >
> > @alexr, can you please advise what is the best way for the "alert and 
> > under quota situation" check in your mind here?
> > 
> > Given that there is only one agent, there is little we could do in such 
> > a case.

I think we should punt on this for now. Originally, we were thinking about 
providing some sort of an explicit alert when allocation goes below its quota, 
but decided not to due to difficulties in determining whether to alert or not 
during failover. I'd suggest we remove this test and the corresponding TODO 
entry altogether for now.


- Alexander


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


On Dec. 27, 2016, 11:24 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Dec. 27, 2016, 11:24 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-3982
> https://issues.apache.org/jira/browse/MESOS-3982
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Dec. 27, 2016, 11:24 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


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


Repository: mesos


Description
---

This implements some tests in previous todos for
quota functionality.

The one for:

`Role quota is below its allocation (InverseOffer generation).`

does not seem be implementable right now, since
hierarchical allocator does not send InverseOffer for
quota yet.


Diffs
-

  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 53691: Implemented some quota functionality tests.

[Mesos ReviewBot]

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



Patch looks great!

Reviews applied: [52284, 53679, 53691]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' 
CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; 
./support/docker_build.sh

- Mesos ReviewBot


On Nov. 12, 2016, 7:59 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Nov. 12, 2016, 7:59 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4292
> https://issues.apache.org/jira/browse/MESOS-4292
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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




src/tests/master_quota_tests.cpp (line 1712)


@alexr, can you please advise what is the best way for the "alert and under 
quota situation" check in your mind here?

Given that there is only one agent, there is little we could do in such a 
case.


- Zhitao Li


On Nov. 12, 2016, 7:59 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53691/
> ---
> 
> (Updated Nov. 12, 2016, 7:59 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov and Xiaojian Huang.
> 
> 
> Bugs: MESOS-4292
> https://issues.apache.org/jira/browse/MESOS-4292
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This implements some tests in previous todos for
> quota functionality.
> 
> The one for:
> 
> `Role quota is below its allocation (InverseOffer generation).`
> 
> does not seem be implementable right now, since
> hierarchical allocator does not send InverseOffer for
> quota yet.
> 
> 
> Diffs
> -
> 
>   src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 
> 
> Diff: https://reviews.apache.org/r/53691/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 53691: Implemented some quota functionality tests.

[Zhitao Li]

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

(Updated Nov. 12, 2016, 7:59 a.m.)


Review request for mesos, Alexander Rukletsov and Xiaojian Huang.


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


Repository: mesos


Description
---

This implements some tests in previous todos for
quota functionality.

The one for:

`Role quota is below its allocation (InverseOffer generation).`

does not seem be implementable right now, since
hierarchical allocator does not send InverseOffer for
quota yet.


Diffs
-

  src/tests/master_quota_tests.cpp 48be7406181646c8cc1d169b82a4a4ca71cdf03b 

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


Testing
---


Thanks,

Zhitao Li