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



Test looks good if you want to pull it out and get a ship it on it separately, 
that would be easier.


src/master/quota_handler.cpp
Lines 589-590 (patched)
<https://reviews.apache.org/r/71068/#comment303917>

    Hm.. this is an over estimate of what we're looking for since reservations 
might be included in both consumed and offered?
    
    I suppose over-rescinding is acceptable, but certainly warrants a comment?



src/master/quota_handler.cpp
Lines 595-596 (patched)
<https://reviews.apache.org/r/71068/#comment303921>

    This CHECK will fail in the future with optimistic offers where we don't 
set an allocation info, but I suppose we'll catch that and any others, since we 
won't need to rescind for frameworks with that future capability



src/master/quota_handler.cpp
Lines 602-609 (patched)
<https://reviews.apache.org/r/71068/#comment303919>

    This actually seems somewhat unacceptable, I wonder if we can do something 
with what we have today, e.g.:
    
    ```
    Rescind logic
    
    allocatorUpdatedFuture
      .then(defer(master, []() { Rescind logic })
    ```
    
    That is, we first rescind enough offers from those that are known to the 
master, then, after the allocator is done updating the quota (which means it 
won't enqueue more offers violating the limits), we do another round of 
rescinding. With this logic, it's still possible though for a framework to have 
accepted an offer that arrived after the initial rescind but before the 
allocator finished updating the quota.. so maybe it's not worth it.
    
    Do we have a ticket that captures this issue? Would be good to link all the 
problems that are caused by the current offer ownership model.



src/master/quota_handler.cpp
Lines 623 (patched)
<https://reviews.apache.org/r/71068/#comment303922>

    extra newline?



src/tests/master_quota_tests.cpp
Lines 1775 (patched)
<https://reviews.apache.org/r/71068/#comment303923>

    are rescinded


- Benjamin Mahler


On July 18, 2019, 5:23 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71068/
> -----------------------------------------------------------
> 
> (Updated July 18, 2019, 5:23 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9812
>     https://issues.apache.org/jira/browse/MESOS-9812
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Outstanding offers of a role (including offers allocated to its
> subroles) are rescinded until the sum of its consumed quota and
> outstanding offered resources are below the requested limits.
> 
> The rescind logic here, however, overlooks the race between master
> and the allocator. Namely, there might be pending offers in the master
> mailbox that have yet to be processed. Here, for simplicity, we ignore
> this race. To properly handle this, the plan is to move offer
> management logic into the allocator and rescind offers there.
> 
> Also added a test.
> 
> 
> Diffs
> -----
> 
>   src/master/quota_handler.cpp 2a3ca5640c850ec8a8aca7abed45386b5b5e1125 
>   src/tests/master_quota_tests.cpp 64a9b31a38af23fb16a4afd04f6d3413c7d1ffe9 
> 
> 
> Diff: https://reviews.apache.org/r/71068/diff/3/
> 
> 
> Testing
> -------
> 
> Ran continously without failure.
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>

Reply via email to