You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2017/12/04 16:29:22 UTC

Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

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

Review request for mesos, 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 715650ee9cb15aed1d1e58badf70fc09e26d13c1 
  src/tests/hierarchical_allocator_tests.cpp 0309074bab180be122c9b0074981e6f69c97feee 


Diff: https://reviews.apache.org/r/64304/diff/1/


Testing
-------

Introduced two dedicated tests.

New failed test: HierarchicalAllocatorTest.OfferAncestorReservationsToDescendantChild


Thanks,

Meng Zhu


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/#review193484
-----------------------------------------------------------


Fix it, then Ship it!




Looks good! Just some comments related to comments and naming, no code changes, so I can take care of these for you and get this committed!


src/master/allocator/mesos/hierarchical.cpp
Lines 1596-1602 (patched)
<https://reviews.apache.org/r/64304/#comment272053>

    Looks like we could add the same structure to this comment as I suggested in the previous review.



src/master/allocator/mesos/hierarchical.cpp
Line 1602 (original), 1640 (patched)
<https://reviews.apache.org/r/64304/#comment272048>

    a quantity?



src/master/allocator/mesos/hierarchical.cpp
Lines 1602-1608 (original), 1640-1646 (patched)
<https://reviews.apache.org/r/64304/#comment272050>

    Some symmetry here would be helpful:
    
    roleReservationQuantities vs 
    roleAllocatedReservationQuantities
    
    The maps:
    
    reservationScalarQuantities vs
    allocatedReservationScalarQuantites
    
    Probably it makes sense to be consistent with "quantities" vs "scalarQuantites"?



src/master/allocator/mesos/hierarchical.cpp
Line 1606 (original), 1644 (patched)
<https://reviews.apache.org/r/64304/#comment272049>

    a quantity?



src/master/allocator/mesos/hierarchical.cpp
Lines 1648-1656 (patched)
<https://reviews.apache.org/r/64304/#comment272052>

    Maybe a little bit more of an explanation of why we count reservations?
    
    ```
          // We charge a role against its quota by considering its
          // allocation as well as any unallocated reservations
          // since reservations are bound to the role. In other
          // words, we always consider reservations as consuming
          // quota, regardless of whether they are allocated.
          // The equation used here is:
          //
          //   Consumed Quota = reservations + unreserved allocation
          //                  = reservations + (allocation - allocated reservations)
          //
          // This is a __quantity__ with no meta-data.
          Resources resourcesChargedAgainstQuota =
            roleReservationScalarQuantities +
              (getQuotaRoleAllocatedResources(role) -
                   roleAllocatedReservationScalarQuantities);
    ```



src/master/allocator/mesos/hierarchical.cpp
Lines 1658-1663 (patched)
<https://reviews.apache.org/r/64304/#comment272054>

    How about?
    
    ```
          // If quota for the role is considered satisfied, then we only
          // further allocate reservations for the role.
          //
          // More precisely, we stop allocating unreserved resources if at
          // least one of the resource guarantees is considered consumed.
          // This technique prevents gaming of the quota allocation,
          // see MESOS-6432.
    ```
    
    I feel like "satisfied" is explained in the second paragraph, and this updates that paragram to say "considered consumed" which matches up with the comment about what we consider consumed against quota.



src/master/allocator/mesos/hierarchical.cpp
Lines 1682-1693 (original), 1732-1743 (patched)
<https://reviews.apache.org/r/64304/#comment272055>

    Looks good, thanks for updating this!



src/master/allocator/mesos/hierarchical.cpp
Lines 1791 (patched)
<https://reviews.apache.org/r/64304/#comment272057>

    // Update the tracking of allocated reservations.
    
    Seems less brittle to becoming stale if the variable name changes.



src/master/allocator/mesos/hierarchical.cpp
Lines 1800-1802 (patched)
<https://reviews.apache.org/r/64304/#comment272056>

    Just an observation, unlike the tracked map, this map will contain role entries with empty resources. Don't think we need to write this down, just an inconsistency between the two that may crop up later (e.g. if this map gets moved to a allocator global map).


- Benjamin Mahler


On Dec. 11, 2017, 4:29 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64304/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2017, 4:29 a.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 9d8b6714d060f67d570c5653798e705248781869 
> 
> 
> Diff: https://reviews.apache.org/r/64304/diff/6/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/
-----------------------------------------------------------

(Updated Dec. 10, 2017, 8:29 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 (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp 9d8b6714d060f67d570c5653798e705248781869 


Diff: https://reviews.apache.org/r/64304/diff/6/

Changes: https://reviews.apache.org/r/64304/diff/5-6/


Testing
-------

Introduced two dedicated tests.
make check.


Thanks,

Meng Zhu


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/#review193288
-----------------------------------------------------------



Great to see this getting fixed!

First pass over just the non-testing code. Can you split out the test into a separate patch?


src/master/allocator/mesos/hierarchical.cpp
Lines 1578-1579 (patched)
<https://reviews.apache.org/r/64304/#comment271855>

    s/every allocation/every allocation run/
    
    You might want to clarify that given the other TODO below this one, the plan is to remove the discrepancy between what the sorter considered allocated and what we'd like to count as allocated for fair sharing and quota. i.e. we should be able to simplify this and just retrieve what the role's "allocation" is in the sorter



src/master/allocator/mesos/hierarchical.cpp
Lines 1584-1597 (patched)
<https://reviews.apache.org/r/64304/#comment271856>

    does this need to be a function? Can you just imbed it in your loop below over each quota role? you also wouldn't need the CHECK anymore?



src/master/allocator/mesos/hierarchical.cpp
Lines 1587-1588 (patched)
<https://reviews.apache.org/r/64304/#comment271857>

    s/&//



src/master/allocator/mesos/hierarchical.cpp
Lines 1587-1588 (patched)
<https://reviews.apache.org/r/64304/#comment271871>

    2 space indent



src/master/allocator/mesos/hierarchical.cpp
Lines 1590 (patched)
<https://reviews.apache.org/r/64304/#comment271858>

    maybe allocatedReservedQuantity?



src/master/allocator/mesos/hierarchical.cpp
Lines 1591 (patched)
<https://reviews.apache.org/r/64304/#comment271859>

    s/(/ (/
    
    .values() will copy (currently), can you use foreachvalue?



src/master/allocator/mesos/hierarchical.cpp
Lines 1592-1593 (patched)
<https://reviews.apache.org/r/64304/#comment271860>

    A note on why we need toUnreserved would be helpful for the reader.



src/master/allocator/mesos/hierarchical.cpp
Lines 1592-1593 (patched)
<https://reviews.apache.org/r/64304/#comment271870>

    2 space indent



src/master/allocator/mesos/hierarchical.cpp
Lines 1599-1601 (patched)
<https://reviews.apache.org/r/64304/#comment271861>

    Let's call it quantities then?
    
    roleAllocatedReservedQuantity
    
    (not sure you need "roles" here)



src/master/allocator/mesos/hierarchical.cpp
Lines 1611 (patched)
<https://reviews.apache.org/r/64304/#comment271862>

    slave



src/master/allocator/mesos/hierarchical.cpp
Lines 1618-1621 (patched)
<https://reviews.apache.org/r/64304/#comment271846>

    s/name/role/ and 2 space indents



src/master/allocator/mesos/hierarchical.cpp
Lines 1602-1609 (original), 1649-1667 (patched)
<https://reviews.apache.org/r/64304/#comment271869>

    2 space indent



src/master/allocator/mesos/hierarchical.cpp
Lines 1663 (patched)
<https://reviews.apache.org/r/64304/#comment271851>

    What goes wrong without this being fixed? Is it fixed later in your chain? Seems like you wrote a test for it?



src/master/allocator/mesos/hierarchical.cpp
Lines 1666-1667 (patched)
<https://reviews.apache.org/r/64304/#comment271866>

    How about (x - y) + z?



src/master/allocator/mesos/hierarchical.cpp
Lines 1673 (patched)
<https://reviews.apache.org/r/64304/#comment271867>

    unreserved



src/master/allocator/mesos/hierarchical.cpp
Lines 1808 (patched)
<https://reviews.apache.org/r/64304/#comment271872>

    2 space indent



src/master/allocator/mesos/hierarchical.cpp
Lines 1808-1810 (patched)
<https://reviews.apache.org/r/64304/#comment271876>

    you can take the slaveId by reference to avoid copying it?



src/master/allocator/mesos/hierarchical.cpp
Lines 1811-1813 (patched)
<https://reviews.apache.org/r/64304/#comment271874>

    rolesAllocatedReservedResources[role] +=
              (resources.reserved(role).nonShared() + newShared)
                .createStrippedScalarQuantity().toUnreserved();



src/master/allocator/mesos/hierarchical.cpp
Lines 1813 (patched)
<https://reviews.apache.org/r/64304/#comment271875>

    a comment here as well about why we need toUnreserved would be helpful


- Benjamin Mahler


On Dec. 8, 2017, 9:51 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64304/
> -----------------------------------------------------------
> 
> (Updated Dec. 8, 2017, 9:51 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 9d8b6714d060f67d570c5653798e705248781869 
>   src/tests/hierarchical_allocator_tests.cpp 862f4683da04d37d9fe9f471d6ec9cd7751f39ec 
> 
> 
> Diff: https://reviews.apache.org/r/64304/diff/5/
> 
> 
> Testing
> -------
> 
> Introduced two dedicated tests.
> make check.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/
-----------------------------------------------------------

(Updated Dec. 8, 2017, 1:51 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 (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp 9d8b6714d060f67d570c5653798e705248781869 
  src/tests/hierarchical_allocator_tests.cpp 862f4683da04d37d9fe9f471d6ec9cd7751f39ec 


Diff: https://reviews.apache.org/r/64304/diff/5/

Changes: https://reviews.apache.org/r/64304/diff/4-5/


Testing
-------

Introduced two dedicated tests.
make check.


Thanks,

Meng Zhu


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
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.


Changes
-------

Patch updated. Thanks for the review!


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 (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp 4bc9fb6f787224114f1285937cf979ace46d8ea3 
  src/tests/hierarchical_allocator_tests.cpp 154049c47f2dae2df8d1bb4ddb5c48c478bb3d0e 


Diff: https://reviews.apache.org/r/64304/diff/4/

Changes: https://reviews.apache.org/r/64304/diff/3-4/


Testing (updated)
-------

Introduced two dedicated tests.
make check.


Thanks,

Meng Zhu


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.

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


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/#review192789
-----------------------------------------------------------




src/master/allocator/mesos/hierarchical.cpp
Line 1492 (original), 1492 (patched)
<https://reviews.apache.org/r/64304/#comment271070>

    Feel free to send a small patch for this since it's an indpendent change, I can get it committed quickly.



src/master/allocator/mesos/hierarchical.cpp
Line 2351 (original), 2370 (patched)
<https://reviews.apache.org/r/64304/#comment271069>

    Whoops! Can you put this in the original patch?



src/tests/hierarchical_allocator_tests.cpp
Lines 1270-1271 (patched)
<https://reviews.apache.org/r/64304/#comment271046>

    Reservations should be accounted towards the quota guarantee/limit even if they are currently unallocated.



src/tests/hierarchical_allocator_tests.cpp
Lines 1272 (patched)
<https://reviews.apache.org/r/64304/#comment271049>

    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?



src/tests/hierarchical_allocator_tests.cpp
Lines 1310-1311 (patched)
<https://reviews.apache.org/r/64304/#comment271052>

    This note seems to contradict the test? Is it a copy paste mistake?



src/tests/hierarchical_allocator_tests.cpp
Lines 1321 (patched)
<https://reviews.apache.org/r/64304/#comment271053>

    As a general comment, be sure to start comments with a capital letter and end them with a period, please do a sweep across this patch for other instances. There are also some typos in the comments, so be sure to do a self-review before publishing a patch.



src/tests/hierarchical_allocator_tests.cpp
Lines 1330-1331 (patched)
<https://reviews.apache.org/r/64304/#comment271054>

    This comment seems like a copy/paste mistake? I'm not sure you need a settle here.



src/tests/hierarchical_allocator_tests.cpp
Lines 1347-1348 (patched)
<https://reviews.apache.org/r/64304/#comment271055>

    allocated to the role



src/tests/hierarchical_allocator_tests.cpp
Lines 1355 (patched)
<https://reviews.apache.org/r/64304/#comment271056>

    What is a satisfied reservation? An allocated reservation?



src/tests/hierarchical_allocator_tests.cpp
Lines 1358 (patched)
<https://reviews.apache.org/r/64304/#comment271058>

    I think you mean allocated resources + unallocated reservations?



src/tests/hierarchical_allocator_tests.cpp
Lines 1359 (patched)
<https://reviews.apache.org/r/64304/#comment271059>

    the quota limit



src/tests/hierarchical_allocator_tests.cpp
Lines 1359-1361 (patched)
<https://reviews.apache.org/r/64304/#comment271060>

    These last two sentences were a bit confusing, I think you can do without them if you update the addition above to reflect the lack of double counting?



src/tests/hierarchical_allocator_tests.cpp
Lines 1362 (patched)
<https://reviews.apache.org/r/64304/#comment271061>

    Looks like this is already stated at the top, can you remove this?



src/tests/hierarchical_allocator_tests.cpp
Lines 1363 (patched)
<https://reviews.apache.org/r/64304/#comment271062>

    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



src/tests/hierarchical_allocator_tests.cpp
Lines 1401 (patched)
<https://reviews.apache.org/r/64304/#comment271064>

    Ditto, this looks like a copy/paste mistake?



src/tests/hierarchical_allocator_tests.cpp
Lines 4737-4796 (original), 4902-4956 (patched)
<https://reviews.apache.org/r/64304/#comment271063>

    Can you pull this bit out into a separate patch? We can get this committed independently.



src/master/allocator/mesos/hierarchical.cpp
Lines 1543-1556 (patched)
<https://reviews.apache.org/r/64304/#comment271082>

    I think ultimately, the sorter needs to consider reserved resources as "allocated", do you have any TODO related to this or any tickets?
    
    I guess the "unfairly satisfy guarantees" in [MESOS-4527](https://issues.apache.org/jira/browse/MESOS-4527) means that we couldn't close this ticket until we also track the reservations in the sorters, which will affect the code here (i.e. you might not need to track the reservations separately?)
    
    Also, we would need a ticket for reservations breaking fairness for the non-quota roles, I don't think there is one yet.



src/master/allocator/mesos/hierarchical.cpp
Lines 1569-1572 (original), 1588-1594 (patched)
<https://reviews.apache.org/r/64304/#comment271083>

    I'm weary of introducing new terminology here (i.e. "possession" of resources). Maybe `resourcesChargedAgainstQuota`?



src/master/allocator/mesos/hierarchical.cpp
Lines 1645-1653 (original), 1673-1681 (patched)
<https://reviews.apache.org/r/64304/#comment271081>

    Can you add a note here about [MESOS-7099](https://issues.apache.org/jira/browse/MESOS-7099)?



src/master/allocator/mesos/hierarchical.cpp
Lines 1797-1798 (patched)
<https://reviews.apache.org/r/64304/#comment271080>

    How about:
    
    ```
        // TODO(mzhu): Breaking early here means that we may leave reservations
        // unallocated. See MESOS-8293.
    ```


- Benjamin Mahler


On Dec. 4, 2017, 11:18 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64304/
> -----------------------------------------------------------
> 
> (Updated Dec. 4, 2017, 11:18 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 20b908cc891f9f9be3045cd9f8be83a11d37ab78 
>   src/tests/hierarchical_allocator_tests.cpp 0309074bab180be122c9b0074981e6f69c97feee 
> 
> 
> Diff: https://reviews.apache.org/r/64304/diff/3/
> 
> 
> Testing
> -------
> 
> Introduced two dedicated tests.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/
-----------------------------------------------------------

(Updated Dec. 4, 2017, 3:18 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 (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp 20b908cc891f9f9be3045cd9f8be83a11d37ab78 
  src/tests/hierarchical_allocator_tests.cpp 0309074bab180be122c9b0074981e6f69c97feee 


Diff: https://reviews.apache.org/r/64304/diff/3/

Changes: https://reviews.apache.org/r/64304/diff/2-3/


Testing
-------

Introduced two dedicated tests.


Thanks,

Meng Zhu


Re: Review Request 64304: Enforced quota limit in the presence of unallocated reservations.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64304/
-----------------------------------------------------------

(Updated Dec. 4, 2017, 1:48 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Michael Park.


Summary (updated)
-----------------

Enforced quota limit in the presence of unallocated reservations.


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 (updated)
-----

  src/master/allocator/mesos/hierarchical.cpp 715650ee9cb15aed1d1e58badf70fc09e26d13c1 
  src/tests/hierarchical_allocator_tests.cpp 0309074bab180be122c9b0074981e6f69c97feee 


Diff: https://reviews.apache.org/r/64304/diff/2/

Changes: https://reviews.apache.org/r/64304/diff/1-2/


Testing (updated)
-------

Introduced two dedicated tests.


Thanks,

Meng Zhu