You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2017/08/24 08:07:29 UTC

Review Request 61880: Fixed agent downgrades for reservation refinement.

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

Review request for mesos and Jiang Yan Xu.


Bugs: MESOS-7714
    https://issues.apache.org/jira/browse/MESOS-7714


Repository: mesos


Description
-------

Previously, `checkpoint(path, resources)` was overloaded such that it
would automatically downgrade the resources before being checkpointed
on the agent. However, `checkpoint(path, protobuf_containing_resources)`
did not work correctly since we didn't recursively look within
the messages to downgrade the resources. Ideally, we would use
protobuf reflection to ensure that these are handled automatically.
For now, we attempt to get all of the places where resources are
present within a message.


Diffs
-----

  src/slave/slave.cpp eac896c43ca0d822f94ed853107b1a9e99d7e05d 
  src/slave/state.hpp 18c43193349ca6ec3d18967dcee69324a435e2fa 
  src/tests/slave_recovery_tests.cpp 9aa0a510d1baad9aea13c03229816ca7c661a37c 


Diff: https://reviews.apache.org/r/61880/diff/1/


Testing
-------

### Before
1. 1. Ran a modified persistent volume framework that doesn't stop after the tasks finish.
2. Downgrading the agent from master to 1.3.1.
3. Observed that the agent cannot start due to resources being incompatible.

### After
1. Ran a modified persistent volume framework that doesn't stop after the tasks finish.
2. Downgrading the agent from master to 1.3.1.
3. Observed that the agent starts successfully.


Thanks,

Michael Park


Re: Review Request 61880: Fixed agent downgrades for reservation refinement.

Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/61880/#review183803
-----------------------------------------------------------


Ship it!





src/slave/slave.cpp
Lines 3457-3458 (patched)
<https://reviews.apache.org/r/61880/#comment259849>

    Here and below, for now is it helpful to just log the result? I guess the reason for not logging anything is that this would be an expected/non-failure case but the return type `Error` suggests it is an error. Just feels like it's not as clean as it could be. Of course I understand we'll refactor this later anyway.


- Jiang Yan Xu


On Aug. 24, 2017, 1:07 a.m., Michael Park wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/61880/
> -----------------------------------------------------------
> 
> (Updated Aug. 24, 2017, 1:07 a.m.)
> 
> 
> Review request for mesos and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7714
>     https://issues.apache.org/jira/browse/MESOS-7714
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Previously, `checkpoint(path, resources)` was overloaded such that it
> would automatically downgrade the resources before being checkpointed
> on the agent. However, `checkpoint(path, protobuf_containing_resources)`
> did not work correctly since we didn't recursively look within
> the messages to downgrade the resources. Ideally, we would use
> protobuf reflection to ensure that these are handled automatically.
> For now, we attempt to get all of the places where resources are
> present within a message.
> 
> 
> Diffs
> -----
> 
>   src/slave/slave.cpp eac896c43ca0d822f94ed853107b1a9e99d7e05d 
>   src/slave/state.hpp 18c43193349ca6ec3d18967dcee69324a435e2fa 
>   src/tests/slave_recovery_tests.cpp 9aa0a510d1baad9aea13c03229816ca7c661a37c 
> 
> 
> Diff: https://reviews.apache.org/r/61880/diff/1/
> 
> 
> Testing
> -------
> 
> ### Before
> 1. 1. Ran a modified persistent volume framework that doesn't stop after the tasks finish.
> 2. Downgrading the agent from master to 1.3.1.
> 3. Observed that the agent cannot start due to resources being incompatible.
> 
> ### After
> 1. Ran a modified persistent volume framework that doesn't stop after the tasks finish.
> 2. Downgrading the agent from master to 1.3.1.
> 3. Observed that the agent starts successfully.
> 
> 
> Thanks,
> 
> Michael Park
> 
>