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 52534: Dispatch filter expiration twice.
Date Thu, 19 Jan 2017 03:11:13 GMT

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


Fix it, then Ship it!




This adjusts the code to fix the problem, so looks good to me.

Longer term it would be great to have something that's less brittle for dealing with the timer
expirations after an allocation on the agent has occurred. The current approach relies on
non-local delay and dispatch knowledge, vs. an approach that was explicitly marking filters
or waiting for a future (e.g. `process::collect(delay, allocationHasOccurred)`).


src/master/allocator/mesos/hierarchical.cpp (lines 1078 - 1087)
<https://reviews.apache.org/r/52534/#comment233548>

    The naming here seems backwards? `_expire()` then `expire()`?



src/master/allocator/mesos/hierarchical.cpp (lines 1078 - 1081)
<https://reviews.apache.org/r/52534/#comment233549>

    Hm.. seems like this should be incorporated to the comment above that is explaining when
to expire the filter.
    
    Also, maybe since the explanation is here we can just use a lambda instead of the additional
top level function?


- Benjamin Mahler


On Jan. 12, 2017, 6:56 p.m., Jacob Janco wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/52534/
> -----------------------------------------------------------
> 
> (Updated Jan. 12, 2017, 6:56 p.m.)
> 
> 
> Review request for mesos, Alexander Rukletsov, Benjamin Mahler, and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-6904
>     https://issues.apache.org/jira/browse/MESOS-6904
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> - With an asynchronous `batch()` allocation,
>   this ensures that filters do not expire
>   before the next allocation.
> - This patch should be reverted when allocation
>   occurs on resource recovery.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp a6424d624864155e1c87a28a63b784512c5c8722

>   src/master/allocator/mesos/hierarchical.cpp 91b1ec43940a788459f045ca4a4b82d4e8373bca

> 
> Diff: https://reviews.apache.org/r/52534/diff/
> 
> 
> Testing
> -------
> 
> With https://reviews.apache.org/r/51027/: 
> 
> GTEST_FILTER="-*SmallOfferFilter*" make check -j8
> 
> 
> Thanks,
> 
> Jacob Janco
> 
>


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