mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Mann <g...@mesosphere.io>
Subject Re: Review Request 66568: Dropped combined operations with GROW and SHRINK volumes.
Date Thu, 19 Apr 2018 21:58:21 GMT

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




src/master/master.cpp
Line 4512 (original), 4515 (patched)
<https://reviews.apache.org/r/66568/#comment282883>

    Nit: there's an extra space after the comma.



src/master/master.cpp
Lines 4525 (patched)
<https://reviews.apache.org/r/66568/#comment282888>

    Does it make sense to use `TaskStatus::REASON_RESOURCES_UNKNOWN` here?



src/master/master.cpp
Lines 4528 (patched)
<https://reviews.apache.org/r/66568/#comment282886>

    s/operation/operations/



src/master/master.cpp
Lines 4518-4522 (original), 4543-4547 (patched)
<https://reviews.apache.org/r/66568/#comment282893>

    Hmmm... it's not clear to me why we don't drop other operations here as well. At the moment,
this just means that we lose some logs, but once we land Gaston's patch (https://reviews.apache.org/r/66679/)
dropping non-launch operations will also send operation feedback.
    
    Could you update this block to drop non-launch operations?


- Greg Mann


On April 17, 2018, 10:53 p.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66568/
> -----------------------------------------------------------
> 
> (Updated April 17, 2018, 10:53 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> These two operations are intended to be non-speculative eventually but
> are implemented as speculative right now. To avoid frameworks opt-in to
> dangerous behavior, we require that accept can only contain one
> `GROW_VOLUME` or `SHRINK_VOLUME` and no other operations.
> 
> This is implemented by reuse existing code which already drops `LAUNCH`
> or `LAUNCH_GROUP` with proper reason and message.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 767ad8cfe142b47ef07172bcb2a4fb49fc3e833a 
> 
> 
> Diff: https://reviews.apache.org/r/66568/diff/3/
> 
> 
> Testing
> -------
> 
> Behavior tested in https://reviews.apache.org/r/66569/.
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


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