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 63977: Used the protobuf-reflection-based 'downgradeResources' utility.
Date Tue, 02 Jan 2018 22:50:38 GMT

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


Fix it, then Ship it!





src/slave/slave.cpp
Lines 1294-1296 (original), 1293-1295 (patched)
<https://reviews.apache.org/r/63977/#comment273594>

    Stale? References downgrade



src/slave/slave.cpp
Lines 1538-1543 (patched)
<https://reviews.apache.org/r/63977/#comment273593>

    What's the consequence of sending a partially downgraded result? Maybe we should spell
that out as it's not that obvious to me



src/slave/slave.cpp
Line 1678 (original), 1660 (patched)
<https://reviews.apache.org/r/63977/#comment273592>

    nice!



src/slave/state.hpp
Lines 96 (patched)
<https://reviews.apache.org/r/63977/#comment273590>

    newline between the paragraphs?
    
    ```
      // We ignore the `Try` from `downgradeResources` here because
      // for now, we checkpoint the result either way.
      //
      // TODO(mpark): Do something smarter with the result once
      // something like an agent recovery capability is introduced.
    ```
    
    It's a little non-obvious to me what the implications of this are.. maybe document it?
Does it mean that we checkpoint only partially downgraded and that's fine so long as we don't
downgrade beyond the 1.x version that introduced refinement..?



src/tests/slave_recovery_tests.cpp
Lines 152-155 (patched)
<https://reviews.apache.org/r/63977/#comment273589>

    Do you want a TODO to use a reflection based upgrade here?


- Benjamin Mahler


On Nov. 21, 2017, 6:07 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63977/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2017, 6:07 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8221
>     https://issues.apache.org/jira/browse/MESOS-8221
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> See summary.
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp cf8a22b636210e9f8d326592d8b64eab2c97caa4 
>   src/slave/slave.cpp 00935616c2328e2ad9fc170199298a3630a701ab 
>   src/slave/state.hpp aaf8e5c2895e1cd88172c5bf73f028b629dc12c7 
>   src/tests/slave_recovery_tests.cpp bf2c5fcabdd4c16bdf5de1b641060e38783b8ee8 
> 
> 
> Diff: https://reviews.apache.org/r/63977/diff/2/
> 
> 
> Testing
> -------
> 
> `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>


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