> On 一月 19, 2016, 11:11 a.m., Alexander Rukletsov wrote: > > How about a more succinct summary: "Traversed all roles for quota > > allocation"? > > > > Changes touching allocator are vulnerable to races, especially in tests. > > Please extend the testing (and mention this in the "Testing Done" section) > > with something like like `GTEST_FILTER="*HierarchicalAllocatorTest*" > > ./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`. > > > > I think we can simplify the second test. Your intention is to test the > > scenario, where one role with quota and lower share is saturated while the > > other is not, and check whether the other will get offers, correct? (Btw, > > scenario description is a great comment for a test!). If so, I'd suggest to > > start with two agents, say "1resource", "2resources" and quotas > > "1resource", "4resources". This guarantees that the first role is > > 1)satisfied and 2)has a lower share. Then you add a third agent with, say, > > "2resources" and check that the second role gets resources offered. Do you > > think this is cleaner and more intuitive?
Yes, I will update the test cases as your proposal. > On 一月 19, 2016, 11:11 a.m., Alexander Rukletsov wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 1845 > > <https://reviews.apache.org/r/41769/diff/3/?file=1199267#file1199267line1845> > > > > Do you think this one is better? > > s/MultiQuotaAbsentFramework/MultiQuotaAbsentFrameworks Done > On 一月 19, 2016, 11:11 a.m., Alexander Rukletsov wrote: > > src/tests/hierarchical_allocator_tests.cpp, line 1849 > > <https://reviews.apache.org/r/41769/diff/3/?file=1199267#file1199267line1849> > > > > It's minor, but I think we tend to put numerals at the back of the > > string, something like "quota-role-1". Done - Guangya ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41769/#review115135 ----------------------------------------------------------- On 一月 19, 2016, 10 a.m., Guangya Liu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41769/ > ----------------------------------------------------------- > > (Updated 一月 19, 2016, 10 a.m.) > > > Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus > Ma, and Neil Conway. > > > Bugs: MESOS-4411 > https://issues.apache.org/jira/browse/MESOS-4411 > > > Repository: mesos > > > Description > ------- > > This patch include two parts: > 1) If there are some `non-active roles` in front of active roles after > `quotaRoleSorter`, when the allocator encounter a `non-active role`, the > allocator should not `break` but `continue` to allocate Quota for other > active roles to make sure other roles can get its quotaed resources. > 2) If some role's quota reach its guaranteed value, the allocator should > handle another role but not break. Take the following case: role1 has quota 5 > and got 5, role2 has quota 100 and got 50, the role1 will be put in front of > role2 by the `quotaRoleSorter`, if allocator `break` when found role1 is > satisfied, then role2 will never get its quotaed resources. > > > Diffs > ----- > > src/master/allocator/mesos/hierarchical.cpp > 72e69a0f42dd724713f2a7a75f1b92ef16eb5569 > src/tests/hierarchical_allocator_tests.cpp > 9362dd306497ba01e0f387c3862456cdcac6f863 > > Diff: https://reviews.apache.org/r/41769/diff/ > > > Testing > ------- > > make > make check > GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" > --verbose --gtest_repeat=100 --gtest_shuffle > > > Thanks, > > Guangya Liu > >
