You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Guangya Liu <gy...@gmail.com> on 2015/12/29 13:30:44 UTC
Review Request 41769: Made allocator traverse all roles for quota
allocation.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/
-----------------------------------------------------------
Review request for mesos and Klaus Ma.
Repository: mesos
Description
-------
Made allocator traverse all roles for quota allocation.
Diffs
-----
src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
Diff: https://reviews.apache.org/r/41769/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review113238
-----------------------------------------------------------
@Alex R and Neil, any comments for this? Thanks.
- Guangya Liu
On 十二月 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated 十二月 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115285
-----------------------------------------------------------
Ship it!
Ship It!
- Joris Van Remoortere
On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Joerg Schad <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115232
-----------------------------------------------------------
The central part lgtm, will check the tests tomorrow morning in more detail.
src/tests/hierarchical_allocator_tests.cpp (line 1855)
<https://reviews.apache.org/r/41769/#comment176138>
s/0;/0
src/tests/hierarchical_allocator_tests.cpp (line 1900)
<https://reviews.apache.org/r/41769/#comment176136>
s/0;/0
src/tests/hierarchical_allocator_tests.cpp (line 1901)
<https://reviews.apache.org/r/41769/#comment176135>
s/0;/0
src/tests/hierarchical_allocator_tests.cpp (line 1903)
<https://reviews.apache.org/r/41769/#comment176137>
it 10 times smaller only for mem
src/tests/hierarchical_allocator_tests.cpp (line 1934)
<https://reviews.apache.org/r/41769/#comment176134>
s/2048;/2048
he
- Joerg Schad
On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115265
-----------------------------------------------------------
Ship it!
Ship It!
- Alexander Rukletsov
On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 19, 2016, 12:42 p.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1875
> > <https://reviews.apache.org/r/41769/diff/3/?file=1199267#file1199267line1875>
> >
> > Here's one question after reviewing this test: is there any priority between Quota? e.g. if the first Quota did not statisfied, should we continue to offer resources to other Quota? Should we lay aside resources for the first one? The first one of Quota is considered to be high priority?
I think this is the current quota behavior. If the first quota did not satisfied, allocator will not allocate resources for another quota based on priority with quota role sorter.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115149
-----------------------------------------------------------
On 一月 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated 一月 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 19, 2016, 12:42 p.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1875
> > <https://reviews.apache.org/r/41769/diff/3/?file=1199267#file1199267line1875>
> >
> > Here's one question after reviewing this test: is there any priority between Quota? e.g. if the first Quota did not statisfied, should we continue to offer resources to other Quota? Should we lay aside resources for the first one? The first one of Quota is considered to be high priority?
>
> Guangya Liu wrote:
> I think this is the current quota behavior. If the first quota did not satisfied, allocator will not allocate resources for another quota based on priority with quota role sorter.
>
> Alexander Rukletsov wrote:
> I think your question is: "If there are not enough resources, shall we ensure we deprive quota'ed roles evenly?" In other words, "shall we avoid cases when one quota is fully satisfied, while the other one is starving?". My answer would be "yes", but it's not a blocker. A situation when there are not enough resources in the cluster to satisfy all quotas should be considered exceptional, hence being less fair is not a big deal, I would say.
>
> Definitely let's not fix it here. Feel free to create a separate ticket though.
I've added a comment in https://reviews.apache.org/r/42511/ . Hope it's more clear now.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115149
-----------------------------------------------------------
On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 19, 2016, 12:42 p.m., Klaus Ma wrote:
> > src/tests/hierarchical_allocator_tests.cpp, line 1875
> > <https://reviews.apache.org/r/41769/diff/3/?file=1199267#file1199267line1875>
> >
> > Here's one question after reviewing this test: is there any priority between Quota? e.g. if the first Quota did not statisfied, should we continue to offer resources to other Quota? Should we lay aside resources for the first one? The first one of Quota is considered to be high priority?
>
> Guangya Liu wrote:
> I think this is the current quota behavior. If the first quota did not satisfied, allocator will not allocate resources for another quota based on priority with quota role sorter.
I think your question is: "If there are not enough resources, shall we ensure we deprive quota'ed roles evenly?" In other words, "shall we avoid cases when one quota is fully satisfied, while the other one is starving?". My answer would be "yes", but it's not a blocker. A situation when there are not enough resources in the cluster to satisfy all quotas should be considered exceptional, hence being less fair is not a big deal, I would say.
Definitely let's not fix it here. Feel free to create a separate ticket though.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115149
-----------------------------------------------------------
On Jan. 19, 2016, 12:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 12:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Klaus Ma <kl...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115149
-----------------------------------------------------------
src/tests/hierarchical_allocator_tests.cpp (line 1875)
<https://reviews.apache.org/r/41769/#comment176038>
Here's one question after reviewing this test: is there any priority between Quota? e.g. if the first Quota did not statisfied, should we continue to offer resources to other Quota? Should we lay aside resources for the first one? The first one of Quota is considered to be high priority?
- Klaus Ma
On Jan. 19, 2016, 8:38 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 19, 2016, 8:38 p.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 48acde69b1a2f305b568a7e322a58708063dd30a
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/
-----------------------------------------------------------
(Updated 一月 19, 2016, 12:38 p.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 (updated)
-------
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 (updated)
-----
src/master/allocator/mesos/hierarchical.cpp 48acde69b1a2f305b568a7e322a58708063dd30a
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
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
> 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review115135
-----------------------------------------------------------
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?
src/master/allocator/mesos/hierarchical.cpp (lines 1142 - 1144)
<https://reviews.apache.org/r/41769/#comment175999>
... to do any allocations for this role.
src/master/allocator/mesos/hierarchical.cpp (line 1156)
<https://reviews.apache.org/r/41769/#comment176000>
s/allocations,/allocations for this role,
src/tests/hierarchical_allocator_tests.cpp (lines 1842 - 1844)
<https://reviews.apache.org/r/41769/#comment175997>
How about this:
This test checks that if one role with quota has no frameworks in it, other roles with quota are still offered resources.
src/tests/hierarchical_allocator_tests.cpp (line 1845)
<https://reviews.apache.org/r/41769/#comment175998>
Do you think this one is better?
s/MultiQuotaAbsentFramework/MultiQuotaAbsentFrameworks
src/tests/hierarchical_allocator_tests.cpp (line 1849)
<https://reviews.apache.org/r/41769/#comment176009>
It's minor, but I think we tend to put numerals at the back of the string, something like "quota-role-1".
src/tests/hierarchical_allocator_tests.cpp (line 1856)
<https://reviews.apache.org/r/41769/#comment176002>
Any reason why you don't use the defacto-standard "cpus:2;mem:1024;disk:0"? I think some people may wonder why this test differs from the other.
Same for the next test.
src/tests/hierarchical_allocator_tests.cpp (line 1860)
<https://reviews.apache.org/r/41769/#comment176006>
How about: "Set quota for both roles".
src/tests/hierarchical_allocator_tests.cpp (lines 1862 - 1863)
<https://reviews.apache.org/r/41769/#comment176004>
blank line, please.
src/tests/hierarchical_allocator_tests.cpp (line 1866)
<https://reviews.apache.org/r/41769/#comment176005>
s/`framework`/a framework (no need for backticks)
s/QUOTA_ROLE2/`QUOTA_ROLE2` (mind backticks).
Same for the next test.
src/tests/hierarchical_allocator_tests.cpp (line 1871)
<https://reviews.apache.org/r/41769/#comment176007>
How about "Due to the coarse-grained nature of the allocations, `framework` will get all `agent`'s resources."
src/tests/hierarchical_allocator_tests.cpp (lines 1879 - 1880)
<https://reviews.apache.org/r/41769/#comment176008>
How about "This test checks that if there are multiple roles with quota all of them get enough offers given there are enough resources."
src/tests/hierarchical_allocator_tests.cpp (lines 1892 - 1893)
<https://reviews.apache.org/r/41769/#comment176010>
Same question as above.
src/tests/hierarchical_allocator_tests.cpp (line 1894)
<https://reviews.apache.org/r/41769/#comment176019>
Let's add a comment here about what is the initial state. For example, take a look at `DRFWithQuota` or `QuotaAgainstStarvation` tests.
src/tests/hierarchical_allocator_tests.cpp (line 1895)
<https://reviews.apache.org/r/41769/#comment176017>
We should definitely drag reader's attention to the fact that quota is pretty different. How about: "Quota for `QUOTA_ROLE1` is 10 times smaller than for `QUOTA_ROLE2`."
src/tests/hierarchical_allocator_tests.cpp (lines 1897 - 1898)
<https://reviews.apache.org/r/41769/#comment176011>
Blank line, please.
src/tests/hierarchical_allocator_tests.cpp (lines 1925 - 1937)
<https://reviews.apache.org/r/41769/#comment176020>
Let's prepend this section with a comment. What is your intention here?
src/tests/hierarchical_allocator_tests.cpp (lines 1939 - 1940)
<https://reviews.apache.org/r/41769/#comment176021>
If you want to wait for `addSlave()` to complete, please move it before recover section. Also, if you do not expect any allocations for addSalve, please explain why.
src/tests/hierarchical_allocator_tests.cpp (line 1941)
<https://reviews.apache.org/r/41769/#comment176022>
Let's describe the cluster state here. Example from one of the existing tests:
```
// Total cluster resources (1 agent): cpus=1, mem=512.
// QUOTA_ROLE share = 0.25 (cpus=0.25, mem=128) [quota: cpus=0.25, mem=128]
// framework1 share = 1
// NO_QUOTA_ROLE share = 0.75 (cpus=0.75, mem=384)
// framework2 share = 0
```
src/tests/hierarchical_allocator_tests.cpp (lines 1957 - 1958)
<https://reviews.apache.org/r/41769/#comment176023>
We need explain why.
- Alexander Rukletsov
On Jan. 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 Jan. 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
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-------
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 (updated)
-------
make
make check
GLOG_v=2 ./bin/mesos-tests.sh --gtest_filter="HierarchicalAllocatorTest.*" --verbose --gtest_repeat=100 --gtest_shuffle
Thanks,
Guangya Liu
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review114863
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41769]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Jan. 16, 2016, 12:45 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Jan. 16, 2016, 12:45 p.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
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> 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
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/
-----------------------------------------------------------
(Updated 一月 16, 2016, 12:45 p.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
-------
Made allocator traverse all roles for quota allocation.
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
Thanks,
Guangya Liu
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/
-----------------------------------------------------------
(Updated 一月 16, 2016, 12:44 p.m.)
Review request for mesos, Alexander Rukletsov, Joris Van Remoortere, Klaus Ma, and Neil Conway.
Repository: mesos
Description
-------
Made allocator traverse all roles for quota allocation.
Diffs (updated)
-----
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
Thanks,
Guangya Liu
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 13, 2016, 11:41 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1164-1166
> > <https://reviews.apache.org/r/41769/diff/2/?file=1180221#file1180221line1164>
> >
> > This looks like a bug to me. I can't remember why we have `break` here in the first place. Could you please elaborate why do you think `break` is fine here?
It is a bit tricky here so I add some comments here even though I prefer we use `continue` here.
The reason that it is OK to use `break` is because all of the quota roles are sorted based on starvation, if the roles in quota role sorter is satisfied, we can assume that all roles behind current satisfied role is also satisfied, that's why `break` here. Comments?
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review114184
-----------------------------------------------------------
On 十二月 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated 十二月 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
> On Jan. 13, 2016, 11:41 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1164-1166
> > <https://reviews.apache.org/r/41769/diff/2/?file=1180221#file1180221line1164>
> >
> > This looks like a bug to me. I can't remember why we have `break` here in the first place. Could you please elaborate why do you think `break` is fine here?
>
> Guangya Liu wrote:
> It is a bit tricky here so I add some comments here even though I prefer we use `continue` here.
>
> The reason that it is OK to use `break` is because all of the quota roles are sorted based on starvation, if the roles in quota role sorter is satisfied, we can assume that all roles behind current satisfied role is also satisfied, that's why `break` here. Comments?
I'm not sure it's right. Roles are sorted according to their allocation, hence a role with quota 5 and allocation 5 will come before role with quota 50 and allocation 10. I believe `continue` should be used here. Also imagine a custom sorter is used, which does not guarantee any ordering.
Anyway, I've checked all the tests we have and it seems there is no test where we set quotas for multiple roles. Do you want to add such test or expand an existing one? Also we may want to add some tests around implicit roles and cover the other issue you fix in this test. There is a ticket for this already: MESOS-4292. We have to figure out what exactly we want to test and sync with NeilC.
I'd be happy to help out and review, this looks like a bug!
Could you please also keep Joris and Qian Zhang in the loop? Thanks!
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review114184
-----------------------------------------------------------
On Dec. 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Dec. 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
> On 一月 13, 2016, 11:41 a.m., Alexander Rukletsov wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 1164-1166
> > <https://reviews.apache.org/r/41769/diff/2/?file=1180221#file1180221line1164>
> >
> > This looks like a bug to me. I can't remember why we have `break` here in the first place. Could you please elaborate why do you think `break` is fine here?
>
> Guangya Liu wrote:
> It is a bit tricky here so I add some comments here even though I prefer we use `continue` here.
>
> The reason that it is OK to use `break` is because all of the quota roles are sorted based on starvation, if the roles in quota role sorter is satisfied, we can assume that all roles behind current satisfied role is also satisfied, that's why `break` here. Comments?
>
> Alexander Rukletsov wrote:
> I'm not sure it's right. Roles are sorted according to their allocation, hence a role with quota 5 and allocation 5 will come before role with quota 50 and allocation 10. I believe `continue` should be used here. Also imagine a custom sorter is used, which does not guarantee any ordering.
>
> Anyway, I've checked all the tests we have and it seems there is no test where we set quotas for multiple roles. Do you want to add such test or expand an existing one? Also we may want to add some tests around implicit roles and cover the other issue you fix in this test. There is a ticket for this already: MESOS-4292. We have to figure out what exactly we want to test and sync with NeilC.
>
> I'd be happy to help out and review, this looks like a bug!
>
> Could you please also keep Joris and Qian Zhang in the loop? Thanks!
Yes, you are right, I think that here still should use `continue` here. Will try to add a new test cases for this.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review114184
-----------------------------------------------------------
On 十二月 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated 十二月 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Alexander Rukletsov <ru...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review114184
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp (lines 1164 - 1166)
<https://reviews.apache.org/r/41769/#comment175008>
This looks like a bug to me. I can't remember why we have `break` here in the first place. Could you please elaborate why do you think `break` is fine here?
- Alexander Rukletsov
On Dec. 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Dec. 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review112427
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41769]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 31, 2015, 11:34 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Dec. 31, 2015, 11:34 p.m.)
>
>
> Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/
-----------------------------------------------------------
(Updated 十二月 31, 2015, 11:34 p.m.)
Review request for mesos, Alexander Rukletsov, Klaus Ma, and Neil Conway.
Repository: mesos
Description
-------
Made allocator traverse all roles for quota allocation.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
Diff: https://reviews.apache.org/r/41769/diff/
Testing
-------
make
make check
Thanks,
Guangya Liu
Re: Review Request 41769: Made allocator traverse all roles for quota
allocation.
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41769/#review112164
-----------------------------------------------------------
Patch looks great!
Reviews applied: [41769]
Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh
- Mesos ReviewBot
On Dec. 29, 2015, 12:30 p.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41769/
> -----------------------------------------------------------
>
> (Updated Dec. 29, 2015, 12:30 p.m.)
>
>
> Review request for mesos and Klaus Ma.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Made allocator traverse all roles for quota allocation.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.cpp 7f900c4e024485704d79e57ae22407557598fe6c
>
> Diff: https://reviews.apache.org/r/41769/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>