mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zhitao Li <zhitaoli...@gmail.com>
Subject Re: Review Request 66050: Implemented grow and shrink of persistent volumes.
Date Wed, 28 Mar 2018 21:26:33 GMT


> On March 26, 2018, 5:37 p.m., Chun-Hung Hsiao wrote:
> > src/slave/slave.cpp
> > Lines 4292-4324 (original), 4292-4342 (patched)
> > <https://reviews.apache.org/r/66050/diff/5/?file=1985839#file1985839line4292>
> >
> >     Let's do some refactoring here. There are 3 cases:
> >     
> >     1. For speculative RP operations, the agent needs to apply them so it can proceed,
but the status update is delegated to RP themselves so no need to generate state updates here.
> >     2. For non-speculative RP operations, the agent can completely delegate the
handling to RP.
> >     3. For non-RP operations, the agent needs to apply, checkpoint, and update the
statuses.
> >     
> >     One option I can think of for refactoring is:
> >     1. Make `apply` handle `GROW_VOLUME` and `SHRINK_VOLUME` and return an `OperationStatus`.
For speculative operation, do the actual resource change and return an update with an empty
conversion; for non-speculative operation, return one with the actual conversion.
> >     2. Do the following here:
> >     ```
> >     if (resourceProviderId.isSome()) {
> >       if (protobuf::isSpeculativeOperation(operation.info())) {
> >         apply(operation);
> >       }
> >       return resourceProviderManager.applyOperation(message);
> >     }
> >     
> >     const OperationStatus status = apply(operation);
> >     
> >     ... // The checkpointing logic.
> >     
> >     UpdateOperationStatusMessage update =
> >       protobuf::createUpdateOperationStatusMessage(... status ...);
> >       
> >     updateOperation(operation, update);
> >     ```
> >     Other thoughts on refactoring/improving the readability are also welcome.
> >     
> >     Also, the current agent code doesn't checkpoint operations, which might cause
some reconciliation problem. But I think we can fix this later, just need to think through
the potential problems first.

I'm separating this to a different patch. After we confirm the logic is sound, let's discuss
refactoring.


- Zhitao


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


On March 28, 2018, 11:26 a.m., Zhitao Li wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66050/
> -----------------------------------------------------------
> 
> (Updated March 28, 2018, 11:26 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
>     https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The new offer operations are implemented as non-speculative operations,
> which means they cannot be chained with previous offer operations which
> depend on results of each other.
> 
> 
> Diffs
> -----
> 
>   src/common/protobuf_utils.cpp 141a444534b776a2c90e2a0daf9727cd21e39080 
>   src/common/resources_utils.cpp 9be01c1abd48264e308960f35cc7c2ee8a367518 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.hpp 7c129ceb929596acbb64d37025e055661277e6bf 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
>   src/tests/mesos.hpp 46c271b5c5bedbdabd58b3cdbb82216d55c846bd 
>   src/tests/persistent_volume_tests.cpp 4edf781711d9efdb994114aeb6289b6af750b87a 
>   src/tests/reservation_tests.cpp 5570df2e0b208512d0a0a3079a180e1acfe08f3d 
> 
> 
> Diff: https://reviews.apache.org/r/66050/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>


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