mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 64303: Tracked resource reservations in the allocator.
Date Tue, 12 Dec 2017 01:03:43 GMT

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


Fix it, then Ship it!




Looks good! Thanks for spelling out the plan in the comments.

I left a few minor edit suggestions to the comments below, I can take care of these for you
and get this committed.


src/master/allocator/mesos/hierarchical.hpp
Lines 456-461 (patched)
<https://reviews.apache.org/r/64303/#comment272030>

    As a general suggestion, we try to use a newline between comment paragraphs, so for example:
    
    ```
      // Aggregated resource reservations on all agents tied to a
      // particular role, if any. These are stripped scalar quantities
      // that contain no meta-data. Used for accounting resource
      // reservations for quota limit.
      //
      // Only roles with non-empty reservations will be stored in the map.
    ```



src/master/allocator/mesos/hierarchical.hpp
Lines 560 (patched)
<https://reviews.apache.org/r/64303/#comment272031>

    No need for parens?



src/master/allocator/mesos/hierarchical.hpp
Lines 562-568 (patched)
<https://reviews.apache.org/r/64303/#comment272032>

    Thanks for spelling this out!
    
    Maybe a little structure to help readability?
    
    ```
      // TODO(mzhu): Ideally, we want these helpers to instead track the
      // reservations as *allocated* in the sorters even when the
      // reservations have not been allocated yet. This will help to:
      //
      //   (1) Solve the fairness issue when roles with unallocated
      //       reservations may game the allocator (See MESOS-8299).
      //
      //   (2) Simplify the quota enforcement logic -- the allocator
      //       would no longer need to track reservations separately.
    ```


- Benjamin Mahler


On Dec. 11, 2017, 4:28 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64303/
> -----------------------------------------------------------
> 
> (Updated Dec. 11, 2017, 4:28 a.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Michael Park.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Resource reservations need to be tracked to make
> sure quota limit will not be exceeded in the presence
> of resource reservation.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp aae63962bab1a9b50828875492966d851328ced5

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

> 
> 
> Diff: https://reviews.apache.org/r/64303/diff/3/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


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