mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Klaus Ma" <klaus1982...@gmail.com>
Subject Re: Review Request 42289: Calcuated 'remainingClusterResources' by all activated slaves.
Date Tue, 19 Jan 2016 10:57:59 GMT


> On Jan. 19, 2016, 5:23 p.m., Alexander Rukletsov wrote:
> > Let's tweak some wording and testing and we are good to go!
> > 
> > I liked the initial summary more. IMO a patch should describe the solution, and
not the problem. It's quite opposite for JIRA tickets, hence I'm convinced patches and tickets
should not share the same title. How about "Calcuated 'remainingClusterResources' using all
available agents."
> > 
> > I also think it won't hurt to extend the description and explain why we need this
change. How about
> > "Event-triggered allocations do not include all available agents. If we
> > calculate remaining resources in the cluster using the partial view,
> > we may overlook already laid away resources for quota'ed roles and lay
> > away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> > of resources."
> > 
> > Changes touching allocator are vulnerable to races, especially in tests. Please
extend the testing (and mention this in the "Testing Done" section) with goodies like `GTEST_FILTER="*HierarchicalAllocatorTest*"
./bin/mesos-tests.sh --gtest_shuffle --gtest-repeat`.
> > 
> > Let's add a test for this change. Ideally the test should fail without the change
and pass it with the change. I think Neil has already provided the test in the ticket, could
you please include it?
> 
> Alexander Rukletsov wrote:
>     Maybe even better, you can modify existing `QuotaAbsentFramework` test.

Yes, that's what I'm thinking :). Current patch passed Neil's test in Mac OS, I'll re-check
it in Ubuntu for `./bin/mesos-tests.sh`; it seems perf did not support no-Linux system.


- Klaus


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


On Jan. 19, 2016, 6:55 p.m., Klaus Ma wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/42289/
> -----------------------------------------------------------
> 
> (Updated Jan. 19, 2016, 6:55 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Ben Mahler, Joris Van Remoortere, and
Neil Conway.
> 
> 
> Bugs: MESOS-4102
>     https://issues.apache.org/jira/browse/MESOS-4102
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> __Phenomenon__:
> Quota doesn't allocate resources on slave joining.
> 
> __Root Cause__:
> Event-triggered allocations do not include all available agents. If we
> calculate remaining resources in the cluster using the partial view,
> we may overlook already laid away resources for quota'ed roles and lay
> away more. Hence we may unnecessarily deprive non-quota'ed frameworks
> of resources.
> 
> Refer to AlexR's comments for more detail:
> https://issues.apache.org/jira/browse/MESOS-4102?focusedCommentId=15048495&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-15048495
> 
> __Solution/Fix__:
> Calcuated 'remainingClusterResources' by all activated slaves.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.cpp 48acde69b1a2f305b568a7e322a58708063dd30a

>   src/tests/hierarchical_allocator_tests.cpp 9362dd306497ba01e0f387c3862456cdcac6f863

> 
> Diff: https://reviews.apache.org/r/42289/diff/
> 
> 
> Testing
> -------
> 
> make
> make check
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>


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