> 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 > >
