mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guangya Liu" <gyliu...@gmail.com>
Subject Re: Review Request 41769: Made allocator traverse all roles for quota allocation.
Date Tue, 19 Jan 2016 12:37:08 GMT


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


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