mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Meng Zhu <m...@mesosphere.io>
Subject Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.
Date Tue, 05 Dec 2017 22:43:05 GMT


> On Dec. 4, 2017, 5:08 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1272 (patched)
> > <https://reviews.apache.org/r/64304/diff/2/?file=1908344#file1908344line1272>
> >
> >     After reading through the test, this is the dynamic reservation case only? Should
we call this test `QuotaLimitWithDynamicReservation` and have another test for static reservations?
Or do you want to test both cases in this test?

Tests parameterized.


> On Dec. 4, 2017, 5:08 p.m., Benjamin Mahler wrote:
> > src/tests/hierarchical_allocator_tests.cpp
> > Lines 1363 (patched)
> > <https://reviews.apache.org/r/64304/diff/2/?file=1908344#file1908344line1363>
> >
> >     This test is also just dynamic reservations, so the same comment above applies.
Maybe we need to parameterize these tests based on whether to use static or dynamic reservation?
> >     
> >     It's also not clear to me why we need both tests, instead of just a single test.
> >     
> >     E.g.
> >     
> >     quota = R
> >     add agent 1 with R resources, reserve them, decline forever
> >     add agent 2 with R resources, they should not be allocated
> >     
> >     revive
> >     
> >     should only get agent 1 resources offered
> >     trigger another allocation cycle (to make sure it's enforced across cycles)
> >     agent 2 should not be offered to the role

Parameterized the test for both dynamic reservation and static reservation.

The first test ensures that quota limit does not get exceeded in the presence of unallocated-reservation.
(expect no pending allocation)

The second test ensures that when a role’s reserved resources gets allocated and (by that
time) if its quota still has not been fully satisfied, it can get unreserved resources to
meet its quota. (expect allocation of unreserved resources).

I do not think you proposed test checks that. I clarified the comments and test name.


- Meng


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


On Dec. 5, 2017, 2:42 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64304/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2017, 2:42 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael Park.
> 
> 
> Bugs: MESOS-4527
>     https://issues.apache.org/jira/browse/MESOS-4527
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Enforced quota limit in the presence of unallocated reservations.
> Also modify the allocation process such that the first stage allocation
> is solely for quota roles while the second stage is solely for
> non-quota roles.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 4bc9fb6f787224114f1285937cf979ace46d8ea3

>   src/tests/hierarchical_allocator_tests.cpp 154049c47f2dae2df8d1bb4ddb5c48c478bb3d0e

> 
> 
> Diff: https://reviews.apache.org/r/64304/diff/4/
> 
> 
> Testing
> -------
> 
> Introduced two dedicated tests.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message