You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jan Schlicht <ja...@mesosphere.io> on 2017/10/18 13:57:07 UTC

Review Request 63107: Added operation feedback for storage operations.

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

Review request for mesos, Benjamin Bannier and Jie Yu.


Repository: mesos


Description
-------

When a framework accepts an offer that contains resource provider
resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
`CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
be guessed and will only be known after the operation has been
successfully applied by the resource provider.
This patch introduces the necessary handling for such operations. The
internal bookkeeping of available resources in the master and agent has
been updated to update resources only after operation feedback has been
received. This ensures that converted resources can only be offered
after the operation was executed by a resource provider.


Diffs
-----

  src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 


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


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188845
-----------------------------------------------------------



Bad patch!

Reviews applied: [63107, 63106, 61947, 63105, 61946, 61810, 63104, 63094, 62903, 63001]

Failed command: python support/apply-reviews.py -n -r 63107

Error:
2017-10-20 16:16:20 URL:https://reviews.apache.org/r/63107/diff/raw/ [13052/13052] -> "63107.patch" [1]
error: patch failed: src/resource_provider/message.hpp:18
error: src/resource_provider/message.hpp: patch does not apply
error: patch failed: src/slave/slave.cpp:6644
error: src/slave/slave.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19860/console

- Mesos Reviewbot


On Oct. 20, 2017, 5:56 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2017, 5:56 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188925
-----------------------------------------------------------



FAIL: Failed to apply the dependent review: 63094.

Failed command: `python.exe .\support\apply-reviews.py -n -r 63094`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107

Relevant logs:

- [apply-review-63094-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107/logs/apply-review-63094-stdout.log):

```
error: patch failed: include/mesos/resource_provider/resource_provider.proto:52
error: include/mesos/resource_provider/resource_provider.proto: patch does not apply
error: patch failed: include/mesos/v1/resource_provider/resource_provider.proto:52
error: include/mesos/v1/resource_provider/resource_provider.proto: patch does not apply
error: patch failed: src/messages/messages.proto:637
error: src/messages/messages.proto: patch does not apply
error: patch failed: src/tests/resource_provider_manager_tests.cpp:286
error: src/tests/resource_provider_manager_tests.cpp: patch does not apply
error: patch failed: src/tests/resource_provider_validation_tests.cpp:98
error: src/tests/resource_provider_validation_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 20, 2017, 12:56 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2017, 12:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/
-----------------------------------------------------------

(Updated Oct. 20, 2017, 2:56 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


Changes
-------

Rebased and addresses issues.


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


Repository: mesos


Description
-------

When a framework accepts an offer that contains resource provider
resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
`CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
be guessed and will only be known after the operation has been
successfully applied by the resource provider.
This patch introduces the necessary handling for such operations. The
internal bookkeeping of available resources in the master and agent has
been updated to update resources only after operation feedback has been
received. This ensures that converted resources can only be offered
after the operation was executed by a resource provider.


Diffs (updated)
-----

  src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 


Diff: https://reviews.apache.org/r/63107/diff/2/

Changes: https://reviews.apache.org/r/63107/diff/1-2/


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188826
-----------------------------------------------------------




src/master/master.cpp
Lines 7088-7089 (patched)
<https://reviews.apache.org/r/63107/#comment265826>

    I need to update the allocator with `updateSlave` here to inform it about the new resources. Otherwise these new resources would never appear in an offer. That's also the reason why `slave->totalResources` has to be changed.



src/resource_provider/message.hpp
Line 36 (original), 37 (patched)
<https://reviews.apache.org/r/63107/#comment265825>

    I'll rebase on https://reviews.apache.org/r/62903/, it'll be `UPDATE_STATE` then.


- Jan Schlicht


On Oct. 18, 2017, 4:39 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2017, 4:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.

> On Oct. 19, 2017, 3:08 a.m., Jie Yu wrote:
> > src/master/master.cpp
> > Lines 7088-7089 (patched)
> > <https://reviews.apache.org/r/63107/diff/1/?file=1862332#file1862332line7088>
> >
> >     This shouldn't be necessary.

I need to update the allocator with updateSlave here to inform it about the new resources. Otherwise these new resources would never appear in an offer. That's also the reason why slave->totalResources has to be changed.


> On Oct. 19, 2017, 3:08 a.m., Jie Yu wrote:
> > src/resource_provider/message.hpp
> > Line 36 (original), 37 (patched)
> > <https://reviews.apache.org/r/63107/diff/1/?file=1862334#file1862334line37>
> >
> >     Not yours, this should probably be `UpdateState`

I'll rebase on https://reviews.apache.org/r/62903/, it'll be UPDATE_STATE then.


- Jan


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


On Oct. 20, 2017, 2:56 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 20, 2017, 2:56 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp a3c9530e340b14b739da90851227f8eed254b4ac 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp d8477b4e364270f5ad61311aa8fe48f823bc7aac 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188618
-----------------------------------------------------------



Not a complete review yet. Will take look again once existing issues are addressed.


src/master/master.cpp
Line 4524 (original), 4526 (patched)
<https://reviews.apache.org/r/63107/#comment265621>

    We probably needs to send `OfferOperation` too here?



src/master/master.cpp
Lines 5103-5107 (original), 5105-5109 (patched)
<https://reviews.apache.org/r/63107/#comment265615>

    We shouldn't use `Resources::apply` here. `apply` to me is to apply a conversion. This is basically consume resources! Follow your logic, why we don't do anything for LAUNCH? is it consuming resources too?
    
    The following `slave->apply` is problematic. It'll reduce slave's `totalResources`?



src/master/master.cpp
Line 5111 (original), 5113 (patched)
<https://reviews.apache.org/r/63107/#comment265616>

    Again, I won't call it "Applying", i would call it "Processing"



src/master/master.cpp
Lines 5126 (patched)
<https://reviews.apache.org/r/63107/#comment265619>

    I think putting operations in each Framework struct makes more sense. It'll help us to see all pending operations from a given framework in state.json or in UI in the future.
    
    I'd suggest we have a `Framework::addOfferOperation(...)` method.
    
    ```
    Framework::addOfferOperation(
        const Offer.Operation& info,
        const string& uuid)
    ```



src/master/master.cpp
Line 5157 (original), 5161 (patched)
<https://reviews.apache.org/r/63107/#comment265613>

    This will update `checkpointedResources`, even if this operation is for a resource provider provided resources. Do we want to do that? I think `checkpointedResources` should be limited to agent default resources (no resource provider).



src/master/master.cpp
Lines 7080 (patched)
<https://reviews.apache.org/r/63107/#comment265622>

    Do we want to set this if it's old operations like RESERVE?



src/master/master.cpp
Lines 7083 (patched)
<https://reviews.apache.org/r/63107/#comment265624>

    slave->totalResources should not change (just like launching a task does not change slave's total).
    
    you should do a Resource::apply here for new operations with additional `converted_resources`:
    
    ```
    Resources::apply(
        const Offer.Operation& operation,
        const Option<Resources>& converted_resources = None())
    ```



src/master/master.cpp
Lines 7085 (patched)
<https://reviews.apache.org/r/63107/#comment265625>

    Let's make sure `checkpointedResources` is only for agent default resources. We only need that because the new master old agent case (depends on agent capabilities, we'll still need to send CheckpointResourcesMessage



src/master/master.cpp
Lines 7088-7089 (patched)
<https://reviews.apache.org/r/63107/#comment265627>

    This shouldn't be necessary.



src/resource_provider/manager.cpp
Lines 318-324 (original), 319-325 (patched)
<https://reviews.apache.org/r/63107/#comment265628>

    Hum, shouldn't we have a validation function for this? I think a single operation cannot operate on resources from different providers.



src/resource_provider/manager.cpp
Lines 329-332 (original), 330-333 (patched)
<https://reviews.apache.org/r/63107/#comment265629>

    We probably need to maintain state for resource providers. What if a resource provider is temporarily disconnected?
    
    Chatted with CHun on this yesterday. I think it makes sense to decouple RP regisration from RP reporting resources.
    
    As a result, each RP will have `CONNECTED` as well as `READY` state.
    
    CONNECTED means that the RP subscribed, but hasn't reported any resources yet. Operation should be dropped in that case.



src/resource_provider/manager.cpp
Line 330 (original), 331 (patched)
<https://reviews.apache.org/r/63107/#comment265630>

    We need to log any operation dropping in the log.



src/resource_provider/manager.cpp
Lines 344-345 (original), 345-346 (patched)
<https://reviews.apache.org/r/63107/#comment265631>

    This is the same as dropping an operation. Please log the operation.



src/resource_provider/message.hpp
Line 36 (original), 37 (patched)
<https://reviews.apache.org/r/63107/#comment265633>

    Not yours, this should probably be `UpdateState`



src/resource_provider/message.hpp
Lines 46 (patched)
<https://reviews.apache.org/r/63107/#comment265634>

    I'd call it `UpdateOfferOperationStatus`


- Jie Yu


On Oct. 18, 2017, 2:39 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2017, 2:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188506
-----------------------------------------------------------



FAIL: Failed to apply the dependent review: 62655.

Failed command: `python.exe .\support\apply-reviews.py -n -r 62655`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107

Relevant logs:

- [apply-review-62655-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63107/logs/apply-review-62655-stdout.log):

```
error: patch failed: src/master/master.cpp:6782
error: src/master/master.cpp: patch does not apply
error: patch failed: src/messages/messages.proto:610
error: src/messages/messages.proto: patch does not apply
error: patch failed: src/slave/slave.cpp:1264
error: src/slave/slave.cpp: patch does not apply
error: patch failed: src/tests/oversubscription_tests.cpp:323
error: src/tests/oversubscription_tests.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 18, 2017, 5:39 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2017, 5:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/#review188571
-----------------------------------------------------------



Bad patch!

Reviews applied: [63107, 63106, 61947, 63105, 61946, 61810, 63104, 63001, 61183, 62158, 62655]

Failed command: python support/apply-reviews.py -n -r 63107

Error:
2017-10-18 20:27:48 URL:https://reviews.apache.org/r/63107/diff/raw/ [12456/12456] -> "63107.patch" [1]
error: patch failed: src/resource_provider/message.hpp:18
error: src/resource_provider/message.hpp: patch does not apply
error: patch failed: src/slave/slave.cpp:6660
error: src/slave/slave.cpp: patch does not apply

Full log: https://builds.apache.org/job/Mesos-Reviewbot/19839/console

- Mesos Reviewbot


On Oct. 18, 2017, 2:39 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63107/
> -----------------------------------------------------------
> 
> (Updated Oct. 18, 2017, 2:39 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Jie Yu.
> 
> 
> Bugs: MESOS-8087
>     https://issues.apache.org/jira/browse/MESOS-8087
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> When a framework accepts an offer that contains resource provider
> resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
> `CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
> be guessed and will only be known after the operation has been
> successfully applied by the resource provider.
> This patch introduces the necessary handling for such operations. The
> internal bookkeeping of available resources in the master and agent has
> been updated to update resources only after operation feedback has been
> received. This ensures that converted resources can only be offered
> after the operation was executed by a resource provider.
> 
> 
> Diffs
> -----
> 
>   src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
>   src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
>   src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
>   src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 
> 
> 
> Diff: https://reviews.apache.org/r/63107/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 63107: Added operation feedback for storage operations.

Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63107/
-----------------------------------------------------------

(Updated Oct. 18, 2017, 4:39 p.m.)


Review request for mesos, Benjamin Bannier and Jie Yu.


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


Repository: mesos


Description
-------

When a framework accepts an offer that contains resource provider
resources with a storage operation (`CREATE_VOLUME`, `DESTROY_VOLUME`,
`CREATE_BLOCK`, `DESTROY_BLOCK`), the result of this operation cannot
be guessed and will only be known after the operation has been
successfully applied by the resource provider.
This patch introduces the necessary handling for such operations. The
internal bookkeeping of available resources in the master and agent has
been updated to update resources only after operation feedback has been
received. This ensures that converted resources can only be offered
after the operation was executed by a resource provider.


Diffs
-----

  src/master/master.hpp 0ddc98259f64b3921d08f5f4ec81543bb0826378 
  src/master/master.cpp 3603878f02ae3dba82811a4a5770dd21ec790ef6 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/message.hpp 3c7c3f2baeb726e04edd6ffbb9784699d7afe521 
  src/slave/slave.hpp aea1e948209c7c8945665915bc2f6d8eb47814ef 
  src/slave/slave.cpp 4d7dc8e9a3901b00103031e24e5d6328d0f2e2ad 


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


Testing
-------

make check


Thanks,

Jan Schlicht