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/09/22 11:26:13 UTC

Review Request 62502: Added PUBLISH and UNPUBLISH events.

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

Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.


Repository: mesos


Description
-------

Before launching a task that use resource provider resources, the Mesos
master will instruct the resource provider to make these resources
available for the task. Also the resource provider will checkpoint
informations to be able to reconcile resource usage in case of a
failover.


Diffs
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
  src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 


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


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

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

> On Oct. 12, 2017, 1:37 a.m., Chun-Hung Hsiao wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 65 (patched)
> > <https://reviews.apache.org/r/62502/diff/1/?file=1832832#file1832832line65>
> >
> >     Is this `uuid` for RP to acknowledge this `Publish` event? Then should we add the corresponding ack calls as well? Ditto below for `Unpublish`.

Yes, exactly that! I wanted to add the ACK calls in a later review, but I agree that's it's better to put them in this one as this will give more context. Will update this patch accordingly.


- Jan


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


On Sept. 22, 2017, 1:26 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 1:26 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review187703
-----------------------------------------------------------




include/mesos/resource_provider/resource_provider.proto
Lines 55 (patched)
<https://reviews.apache.org/r/62502/#comment264764>

    After discussed with Jie, we think it's OK to overload `PUBLISH` and `UNPUBLISH` for RP masters and RP agents.



include/mesos/resource_provider/resource_provider.proto
Lines 65 (patched)
<https://reviews.apache.org/r/62502/#comment264750>

    Is this `uuid` for RP to acknowledge this `Publish` event? Then should we add the corresponding ack calls as well? Ditto below for `Unpublish`.


- Chun-Hung Hsiao


On Sept. 22, 2017, 11:26 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

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



PASS: Mesos patch 62502 was successfully built and tested.

Reviews applied: `['62502']`

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

- Mesos Reviewbot Windows


On Sept. 22, 2017, 11:26 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

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



Patch looks great!

Reviews applied: [62502]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On Sept. 22, 2017, 11:26 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

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



FAIL: Failed to apply the dependent review: 62282.

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

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

Relevant logs:

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

```
error: patch failed: src/common/resources.cpp:174
error: src/common/resources.cpp: patch does not apply
```

- Mesos Reviewbot Windows


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review189512
-----------------------------------------------------------




include/mesos/v1/resource_provider/resource_provider.proto
Lines 115 (patched)
<https://reviews.apache.org/r/62502/#comment266626>

    How can a resource provider tell the manager that the publish is failed?


- Chun-Hung Hsiao


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

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



Bad patch!

Reviews applied: [62502, 61947, 61946, 61810, 58021, 58047, 58048, 62282]

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

Error:
2017-10-18 08:59:42 URL:https://reviews.apache.org/r/61947/diff/raw/ [8417/8417] -> "61947.patch" [1]
error: patch failed: src/tests/resource_provider_manager_tests.cpp:339
error: src/tests/resource_provider_manager_tests.cpp: patch does not apply

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

- Mesos Reviewbot


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added an event to publish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review191607
-----------------------------------------------------------




include/mesos/resource_provider/resource_provider.proto
Lines 141 (patched)
<https://reviews.apache.org/r/62502/#comment269451>

    I think we need a way to tell the RP manager if the publish request is successful or not. Also I checked our naming convention and it seems "Acknowledge" is not the best name for this (my bad). How about the following?
    
    ```
    message UpdatePublishStatus {
      enum State {
        UNKNOWN = 0;
        OK = 1;
        FAILED = 2;
      }
      
      required bytes uuid = 1;
      required State state = 2;
      optional string message = 3;
    }
    ```



include/mesos/v1/resource_provider/resource_provider.proto
Lines 141 (patched)
<https://reviews.apache.org/r/62502/#comment269452>

    Ditto.


- Chun-Hung Hsiao


On Nov. 21, 2017, 11:55 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Nov. 21, 2017, 11:55 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'
> operations by sending back 'ACKNOWLEDGE_PUBLISH' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto 1e44b952691fa57b546e979bc5876df3d82d746f 
>   include/mesos/v1/resource_provider/resource_provider.proto 3956f82f734458bc3c7ee6813f2784f3d4767cd8 
>   src/resource_provider/manager.cpp 6dfc42900a1e4249f37cec585f7fe50f5aa94e43 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp ddc57c3eac70c4b105785f214357e27d84a93332 
>   src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added an event to publish resources.

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

(Updated Nov. 21, 2017, 12:55 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.


Changes
-------

Addressed issues.


Summary (updated)
-----------------

Added an event to publish resources.


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


Repository: mesos


Description (updated)
-------

Before launching a task that use resource provider resources, the Mesos
master will instruct the resource provider to make these resources
available for the task. External resource providers will checkpoint
informations to be able to reconcile resource usage in case of a
failover. Resource providers will acknowledge the 'PUBLISH'
operations by sending back 'ACKNOWLEDGE_PUBLISH' together with
the UUID of the operation.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto 1e44b952691fa57b546e979bc5876df3d82d746f 
  include/mesos/v1/resource_provider/resource_provider.proto 3956f82f734458bc3c7ee6813f2784f3d4767cd8 
  src/resource_provider/manager.cpp 6dfc42900a1e4249f37cec585f7fe50f5aa94e43 
  src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
  src/resource_provider/validation.cpp ddc57c3eac70c4b105785f214357e27d84a93332 
  src/tests/mesos.hpp 345b883a8c629bf5bed83e9236632c277f2eb0eb 


Diff: https://reviews.apache.org/r/62502/diff/3/

Changes: https://reviews.apache.org/r/62502/diff/2-3/


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On Oct. 17, 2017, 8:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line38>
> >
> >     No need for `UNPUBLISH` for now. Let's introduce it once we actually need it.
> 
> Chun-Hung Hsiao wrote:
>     We may still want `UNPUBLISH`, depending on what we want to implement in the following case.
>     A storage local resource provider needs to call `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`.
>     We can either:
>     1. Ask the storage local resource provider to checkpoint what's being published, and do the proper unpublish call before deleting. This requires no `UNPUBLISH`.
>     2. Ask the resource provider manager to checkpoint what's being published, and it is responsible to issue an `UNPUBLISH` before sending out the `DestroyVolume` offer operation.
>     Which one sounds better? Also if we consider general resource providers, should the manager or the provider checkpoint the published resources?
> 
> Chun-Hung Hsiao wrote:
>     Now I think since the RP need to remember what is published, because we don't do `UBPUBLISH` for local resources for now, I'll just make the RP do it's own unpublish.

If we want to keep the concept of "resource usage" and that of "resource publish" decoupled and maintain the same "ensure total" semantics in the future, we probably need `PUBLISH_AT_LEAST` and `PUBLISH_NO_MORE_THAN` instead of `PUBLISH`/`UNPUBLISH`. These operations provide flexibility for us to decide how to do reaource unpublishing (eager/periodic GC/lazy) later. Are there better names for these operations?


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Benjamin Bannier <bb...@apache.org>.

> On Oct. 17, 2017, 10:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line38>
> >
> >     No need for `UNPUBLISH` for now. Let's introduce it once we actually need it.
> 
> Chun-Hung Hsiao wrote:
>     We may still want `UNPUBLISH`, depending on what we want to implement in the following case.
>     A storage local resource provider needs to call `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`.
>     We can either:
>     1. Ask the storage local resource provider to checkpoint what's being published, and do the proper unpublish call before deleting. This requires no `UNPUBLISH`.
>     2. Ask the resource provider manager to checkpoint what's being published, and it is responsible to issue an `UNPUBLISH` before sending out the `DestroyVolume` offer operation.
>     Which one sounds better? Also if we consider general resource providers, should the manager or the provider checkpoint the published resources?
> 
> Chun-Hung Hsiao wrote:
>     Now I think since the RP need to remember what is published, because we don't do `UBPUBLISH` for local resources for now, I'll just make the RP do it's own unpublish.
> 
> Chun-Hung Hsiao wrote:
>     If we want to keep the concept of "resource usage" and that of "resource publish" decoupled and maintain the same "ensure total" semantics in the future, we probably need `PUBLISH_AT_LEAST` and `PUBLISH_NO_MORE_THAN` instead of `PUBLISH`/`UNPUBLISH`. These operations provide flexibility for us to decide how to do reaource unpublishing (eager/periodic GC/lazy) later. Are there better names for these operations?

I am not sure decoupling these on this level is useful, and for simplicity I'd much prefer callers to only have to use a single pair of messages to signal that resources are needed or not needed anymore. Currently it looks like these messages would be used to inform the resource provider about tasks which are being launched and have ended, and we don't currently have or plan to add knobs to parameterize this behavior along the axes you outline. 

Not sure what scenarios you have in mind for `PUBLISH_NO_MORE_THAN` (better: use case for isolation? not sure), but as a user I would already expect operations on the plugin to be idempotent enough so that `PUBLISH_AT_LEAST` wouldn't be needed. I also not sure callers at this level would want to parameterize unpublishing like you hinted; this instead sounds like a use case for plugin configuration to me.


- Benjamin


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


On Oct. 13, 2017, 2:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 2:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On Oct. 17, 2017, 8:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line38>
> >
> >     No need for `UNPUBLISH` for now. Let's introduce it once we actually need it.
> 
> Chun-Hung Hsiao wrote:
>     We may still want `UNPUBLISH`, depending on what we want to implement in the following case.
>     A storage local resource provider needs to call `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`.
>     We can either:
>     1. Ask the storage local resource provider to checkpoint what's being published, and do the proper unpublish call before deleting. This requires no `UNPUBLISH`.
>     2. Ask the resource provider manager to checkpoint what's being published, and it is responsible to issue an `UNPUBLISH` before sending out the `DestroyVolume` offer operation.
>     Which one sounds better? Also if we consider general resource providers, should the manager or the provider checkpoint the published resources?

Now I think since the RP need to remember what is published, because we don't do `UBPUBLISH` for local resources for now, I'll just make the RP do it's own unpublish.


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On Oct. 17, 2017, 8:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 57-58 (patched)
> > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line57>
> >
> >     These two are not no needed for now. Let's remove it for now.

Also, since we don't do unpublish, the current semantics of the `PUBLISH` event is that the agent notifies a resource provider about ALL resources that needs to be in the "published" state. That means the resources may comes from all frameworks and there is no single `framework_id` that we can assign to this event. If we want to keep it the same in the future, then we probably need to change the interface of the resource allocator to avoid passing in `framework_id`s.


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On Oct. 17, 2017, 8:48 p.m., Jie Yu wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 38 (patched)
> > <https://reviews.apache.org/r/62502/diff/2/?file=1854486#file1854486line38>
> >
> >     No need for `UNPUBLISH` for now. Let's introduce it once we actually need it.

We may still want `UNPUBLISH`, depending on what we want to implement in the following case.
A storage local resource provider needs to call `csi::Node::NodeUnpublishVolume` before `csi::Controller::DeleteVolume`.
We can either:
1. Ask the storage local resource provider to checkpoint what's being published, and do the proper unpublish call before deleting. This requires no `UNPUBLISH`.
2. Ask the resource provider manager to checkpoint what's being published, and it is responsible to issue an `UNPUBLISH` before sending out the `DestroyVolume` offer operation.
Which one sounds better? Also if we consider general resource providers, should the manager or the provider checkpoint the published resources?


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

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




include/mesos/resource_provider/resource_provider.proto
Lines 38 (patched)
<https://reviews.apache.org/r/62502/#comment265363>

    No need for `UNPUBLISH` for now. Let's introduce it once we actually need it.



include/mesos/resource_provider/resource_provider.proto
Lines 53-54 (patched)
<https://reviews.apache.org/r/62502/#comment265366>

    Please adjust the comments here. This is for the agent to ask the RP agent to make the resources ready to be used on the agent node.



include/mesos/resource_provider/resource_provider.proto
Lines 57-58 (patched)
<https://reviews.apache.org/r/62502/#comment265365>

    These two are not no needed for now. Let's remove it for now.



include/mesos/resource_provider/resource_provider.proto
Lines 62-67 (patched)
<https://reviews.apache.org/r/62502/#comment265364>

    See comments above. Remove this.



include/mesos/resource_provider/resource_provider.proto
Lines 88 (patched)
<https://reviews.apache.org/r/62502/#comment265368>

    Ditto on removal for now.



include/mesos/resource_provider/resource_provider.proto
Lines 124-126 (patched)
<https://reviews.apache.org/r/62502/#comment265367>

    Ditto on removal for now


- Jie Yu


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review189464
-----------------------------------------------------------




include/mesos/resource_provider/resource_provider.proto
Lines 87 (patched)
<https://reviews.apache.org/r/62502/#comment266564>

    We used to use verbs in `Call` of all other APIs (scheduler, executor, etc). The past perfect tense does not look like a "call". How about `AcknowledgePublish`?


- Chun-Hung Hsiao


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added events to publish and unpublish resources.

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

(Updated Oct. 13, 2017, 2:01 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.


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


Repository: mesos


Description
-------

Before launching a task that use resource provider resources, the Mesos
master will instruct the resource provider to make these resources
available for the task. External resource providers will checkpoint
informations to be able to reconcile resource usage in case of a
failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
the UUID of the operation.


Diffs
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 


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


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 62502: Added events to publish and unpublish resources.

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

(Updated Oct. 13, 2017, 1:20 p.m.)


Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.


Changes
-------

Added `PUBLISHED` and `UNPUBLISHED` responses.


Summary (updated)
-----------------

Added events to publish and unpublish resources.


Repository: mesos


Description (updated)
-------

Before launching a task that use resource provider resources, the Mesos
master will instruct the resource provider to make these resources
available for the task. External resource providers will checkpoint
informations to be able to reconcile resource usage in case of a
failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
the UUID of the operation.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
  src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
  src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
  src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 


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

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


Testing
-------

make check


Thanks,

Jan Schlicht


Re: Review Request 62502: Added events to publish and unpublish resources.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.

> On Sept. 22, 2017, 7:05 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 56 (patched)
> > <https://reviews.apache.org/r/62502/diff/1/?file=1832832#file1832832line56>
> >
> >     Can you elaborate the needs of `uuid` and `framework_id`?

We need the `uuid` for ack, and `framework_id` for ERP to checkpoint the allocation because of the current allocation interface (https://github.com/apache/mesos/blob/master/include/mesos/allocator/allocator.hpp#L342). But, we can consider changing the interface and remove `framework_id` here to keep an RP unaware of frameworks. Thoughts?


- Chun-Hung


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


On Oct. 13, 2017, 12:01 p.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Oct. 13, 2017, 12:01 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Bugs: MESOS-8089
>     https://issues.apache.org/jira/browse/MESOS-8089
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. External resource providers will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover. Resource providers will acknowledge the 'PUBLISH'/'UNPUBLISH'
> operations by sending back 'PUBLISHED'/'UNPUBLISHED' together with
> the UUID of the operation.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0 
>   src/tests/mesos.hpp 24d220e292bc1e137992e8f81484477b62bd0896 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review186007
-----------------------------------------------------------




include/mesos/resource_provider/resource_provider.proto
Lines 56 (patched)
<https://reviews.apache.org/r/62502/#comment262359>

    Can you elaborate the needs of `uuid` and `framework_id`?


- Chun-Hung Hsiao


On Sept. 22, 2017, 11:26 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>


Re: Review Request 62502: Added PUBLISH and UNPUBLISH events.

Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62502/#review186020
-----------------------------------------------------------




include/mesos/resource_provider/resource_provider.proto
Lines 55 (patched)
<https://reviews.apache.org/r/62502/#comment262383>

    It seems to me that me might want to have two types of `Publish` events, one for RP masters and one for RP agents. The RP master publish event would notify the RP master to checkpoint which agent is going to use the resource, but the RP agent publish don't need such information, since the event sender (Mesos master) and the event receiver (Mesos agent) would have already know the agent ID.


- Chun-Hung Hsiao


On Sept. 22, 2017, 11:26 a.m., Jan Schlicht wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62502/
> -----------------------------------------------------------
> 
> (Updated Sept. 22, 2017, 11:26 a.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Chun-Hung Hsiao, and Jie Yu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Before launching a task that use resource provider resources, the Mesos
> master will instruct the resource provider to make these resources
> available for the task. Also the resource provider will checkpoint
> informations to be able to reconcile resource usage in case of a
> failover.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/resource_provider/storage/provider.cpp 46224997430ac0c568904d80014166a6f059907f 
>   src/tests/mesos.hpp b0749db273c91692b91d7ca87f975694b18e422c 
> 
> 
> Diff: https://reviews.apache.org/r/62502/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jan Schlicht
> 
>