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 60564: Performed validation/upgrade of `Resource` objects before authorization.
Date Sat, 01 Jul 2017 07:15:23 GMT

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



Looks good, but I would really like to see the logic for dropping operations in accept() just
once, rather than duplicating it here and the two being inconsistent.


src/master/master.cpp
Lines 3944 (patched)
<https://reviews.apache.org/r/60564/#comment254173>

    It seems to me the `error.isSome()` handling above and the one here should be consistent,
and can be merged into one? Both are responsible for dropping operations.
    
    Note also that the duplication here is inconsistent, for example the above calls forward
and increments dropped/lost metrics, but this one calls sendstatusupdates?



src/tests/master_tests.cpp
Lines 7588 (patched)
<https://reviews.apache.org/r/60564/#comment254174>

    "such" means.. tasks that have post reservation refinement formatted resources?


- Benjamin Mahler


On June 30, 2017, 9:02 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60564/
> -----------------------------------------------------------
> 
> (Updated June 30, 2017, 9:02 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-7735
>     https://issues.apache.org/jira/browse/MESOS-7735
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Performed validation/upgrade of `Resource` objects before authorization.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 95e9691b3e3ef101e3e7ddc7a498d5c5bf2276e3 
>   src/tests/master_tests.cpp cfb799fd105e9880cd56415b2a84e604c8f62703 
> 
> 
> Diff: https://reviews.apache.org/r/60564/diff/2/
> 
> 
> Testing
> -------
> 
> Added a new test + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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