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 72738: Added protobuf messages for constraints-based offer filtering.
Date Tue, 11 Aug 2020 19:16:00 GMT

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


Ship it!




Overall structure looks good! I just left a documentation suggestion, but giving a ship it
anyway and happy to take another look once updated if you like.


include/mesos/scheduler/scheduler.proto
Lines 250 (patched)
<https://reviews.apache.org/r/72738/#comment310611>

    Hm.. seems fine to me, but I do wonder if this is the best name vs say.. FirstClassAttribute



include/mesos/scheduler/scheduler.proto
Lines 264-270 (patched)
<https://reviews.apache.org/r/72738/#comment310612>

    I guess we can put the braces on the same line for these? Seems a bit cleaner



include/mesos/scheduler/scheduler.proto
Lines 273-274 (patched)
<https://reviews.apache.org/r/72738/#comment310613>

    Is there a TODO here to show intent of more being added? (like you have for resources
below)



include/mesos/scheduler/scheduler.proto
Lines 282 (patched)
<https://reviews.apache.org/r/72738/#comment310605>

    Here's a suggestion:
    
    ```
    Frameworks can express offer constraints for their roles.
    
    Constraints restrict which offers will be sent to the framework:
    if an offer does not match the provided constraints, it will not
    be sent to the framework.
    
    Constraints are expressed on a per role basis. If you consider a
    scheduler that has multiple apps to launch within a single role,
    the structure of the constraints for that role looks as follows:
    
            app 1             app 2                          app n
        constraints   OR   constraints   OR   ...   OR   constraints
                               /\
                              /  \
            constraint 1 AND constraint 2 AND ... AND constraint n
    
    That is, one of the constraint groups must match for an offer to
    be generated, and within a group all the constraints must match.
    
    As a concrete example, consider a scheduler with two applications
    with multiple instances it wants to launch within a role:
    
        application 1: hostname == "foo"
        application 2: unique "rack" attribute
        
    Assuming there are already some instances of application 2 launched,
    the constraints might look like the following:
    
            app 1              app 2
        constraints    OR    constraints
             /\                 /\
            /  \               /  \
      hostname == "foo"    rack != "X" AND
                           rack != "Y" AND
                           rack != "Z"
                       
                       ==
                       
              (hostname == "foo")
                       OR
      (rack != "X" AND rack != "Y" AND rack != "Z")
    
    The benefits of expressing constraints are:
    
    (1) reduced amount of unusable offers received, and hence:
    (2) reduced traffic and processing overhead due to unusable
        offer / DECLINE back and forth churn
    (3) most importantly, reduced latency to receive the desired
        offer for a particular task
    ```
    
    Hope that eliminates the need for the lower level comments on RoleConstraints and Group.
Would be good to get some another pair of eyes on this documentation to make sure people can
understand it clearly.
    
    This would also translate pretty cleanly to our website docs?



src/internal/devolve.cpp
Lines 248-249 (original), 248-249 (patched)
<https://reviews.apache.org/r/72738/#comment310610>

    Just to double check, I assume you found this by searching for suppressed_roles or some
other piece of the message?


- Benjamin Mahler


On Aug. 6, 2020, 4:33 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72738/
> -----------------------------------------------------------
> 
> (Updated Aug. 6, 2020, 4:33 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10171
>     https://issues.apache.org/jira/browse/MESOS-10171
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This patch adds framework's offer constraints  into `Subscribe` and
> `UpdateFramework` calls, which is a prerequisite to implementing
> constraints-based offer filtering (see MESOS-10161).
> 
> 
> Diffs
> -----
> 
>   include/mesos/scheduler/scheduler.proto 6e1639a9baf017fa87b274daeedc821389216ddc 
>   include/mesos/v1/scheduler/scheduler.proto eb5fdeb984b28403bd8281742bd0a5f2053863e3

>   src/internal/devolve.cpp 4527c522473b74622055e1765740e3706b95afdb 
> 
> 
> Diff: https://reviews.apache.org/r/72738/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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