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 70132: Do not implicitly decline speculatively converted resources.
Date Fri, 26 Apr 2019 00:05:42 GMT

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


Fix it, then Ship it!




Thank you Chun!

I think the description is still a bit tough of a read, here's a
suggestion to work off of that hopefully makes the issue clearer to
the uninitiated:

```
Currently, if a framework accepts an offer with a RESERVE operation,
the RESERVE resoures will be implicitly declined. This is counter to
what one would expect (that only the remaining resources in the offer
will be declined):

  Offer cpus:10 -> ACCEPT with RESERVE cpus(role):1
                   *Actual* implicit decline: cpus:9;cpus(role):1
                   *Expected* implicit decline: cpus:9

The same issue is present with the other transformational operations
(i.e. UNRESERVE, CREATE, ...).

This patch fixes the issue by computing the transformed "remaining"
resources in the offer and subtracting the converted resources from
the remaining resources. In the example above:

  Offered:   cpus:10
  Remaining: cpus:9;cpus(role):1
  Implicitly Declined = Remaining - (Remaining - Offered)
                      = cpus:9;cpus(role):1 - cpus(role):1
                      = cpus:9
```


src/master/master.cpp
Lines 5941-5950 (patched)
<https://reviews.apache.org/r/70132/#comment301196>

    I think some clear formulas would be more readable over the wall of text?
    
    ```
    // We now need to compute the amounts of resources to (1) decline with the
    // filter and (2) recover without a filter.
    //
    //   Implicity      (Offered Resources).apply(Operations)
    //    Declined  =     - LAUNCH[_GROUP] Resources
    //   Resources        - Converted Remaining Resources
    //
    //                      |
    //                      V
    //
    //   Implicity      _offeredResources
    //    Declined  =     - Converted Remaining Resources
    //   Resources
    //
    //                      |
    //                      V
    //
    //   Implicity      _offeredResources
    //    Declined  =     - (_offeredResources - offeredResources)
    //   Resources
    //
    ```



src/master/master.cpp
Lines 5944-5945 (patched)
<https://reviews.apache.org/r/70132/#comment301194>

    "We should" seems to imply there's no api contract or anything, probably this can just
phrase it as what we're supposed to do and how we compute it?



src/master/master.cpp
Lines 5943-5944 (original), 5963-5964 (patched)
<https://reviews.apache.org/r/70132/#comment301195>

    Let's remove this TODO, I don't think it's accurate. If pausing is a workaround of anything,
it's that we don't allow recovering both pieces of the offer in a single call, which I don't
believe there's a ticket for


- Benjamin Mahler


On April 25, 2019, 10:14 p.m., Chun-Hung Hsiao wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70132/
> -----------------------------------------------------------
> 
> (Updated April 25, 2019, 10:14 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, and Meng Zhu.
> 
> 
> Bugs: MESOS-9616
>     https://issues.apache.org/jira/browse/MESOS-9616
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Currently if a framework accepts an offer to perform pipelined
> operations, e.g., reserving resource, without a final consumer, the
> converted resources will be implicitly declined. This is an undesired
> behavior as the framework might want to reserve one resource first but
> launch a task later in the next allocation cycle. This patch fixes this
> behavior.
> 
> But, if the framework accepts an offers with multiple operations that
> cancel out each other, the resources consumed by these operations are
> still considered unused and will be declined.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp ad54ae217863a08f4e6d743b39c176b171353084 
>   src/tests/slave_tests.cpp b1c3a01031b917fb9773c8c890a8f88838870559 
> 
> 
> Diff: https://reviews.apache.org/r/70132/diff/7/
> 
> 
> Testing
> -------
> 
> make check
> 
> More testing done in r/70537.
> 
> 
> Thanks,
> 
> Chun-Hung Hsiao
> 
>


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