You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jie Yu <yu...@gmail.com> on 2017/10/19 00:10:48 UTC

Re: Review Request 63094: Added resource version uuid for offer operations.

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

(Updated Oct. 19, 2017, 12:10 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Changes
-------

Used UUID and a vector instead. Singular clock idea for the agent does not work. See reasoning in this notes:
https://docs.google.com/document/d/1RrrLVATZUyaURpEOeGjgxA6ccshuLo94G678IbL-Yco/edit#bookmark=id.v893d0oct2wh

This patch is based on https://reviews.apache.org/r/62903/


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

Added resource version uuid for offer operations.


Repository: mesos


Description (updated)
-------

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and changes it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 


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

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


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 63094: Added resource version uuid for offer operations.

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

(Updated Oct. 29, 2017, 3:15 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Bugs: MESOS-7235 and MESOS-8054
    https://issues.apache.org/jira/browse/MESOS-7235
    https://issues.apache.org/jira/browse/MESOS-8054


Repository: mesos


Description
-------

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and change it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63094/diff/5/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 63094: Added resource version uuid for offer operations.

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

(Updated Oct. 29, 2017, 10:37 a.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Changes
-------

Addressed comments.


Repository: mesos


Description (updated)
-------

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and change it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63094/diff/5/

Changes: https://reviews.apache.org/r/63094/diff/4-5/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 63094: Added resource version uuid for offer operations.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review189457
-----------------------------------------------------------


Fix it, then Ship it!





include/mesos/resource_provider/resource_provider.proto
Lines 58 (patched)
<https://reviews.apache.org/r/63094/#comment266557>

    s/changes/change/
    
    Here and elsewhere (also in the description :)



src/messages/messages.proto
Lines 716 (patched)
<https://reviews.apache.org/r/63094/#comment266580>

    Maybe use `ResourceVersionUUID` here instead?


- Greg Mann


On Oct. 27, 2017, 5:06 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> -----------------------------------------------------------
> 
> (Updated Oct. 27, 2017, 5:06 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 63094: Added resource version uuid for offer operations.

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

(Updated Oct. 27, 2017, 5:06 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Changes
-------

Addressed comments.


Repository: mesos


Description
-------

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and changes it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 


Diff: https://reviews.apache.org/r/63094/diff/4/

Changes: https://reviews.apache.org/r/63094/diff/3-4/


Testing
-------

make check


Thanks,

Jie Yu


Re: Review Request 63094: Added resource version uuid for offer operations.

Posted by Jie Yu <yu...@gmail.com>.

> On Oct. 20, 2017, 12:44 p.m., Benjamin Bannier wrote:
> > src/messages/messages.proto
> > Lines 663 (patched)
> > <https://reviews.apache.org/r/63094/diff/3/?file=1864433#file1864433line663>
> >
> >     Even though currently operation on agent resources cannot exhibit operation speculation failures, I would still prefer would we include some version uuid for the agent resources here as well.
> >     
> >     AFAICT this could happen either by adjusting `ResourceVersionUUID` so that `resource_provider_id` becomes `optional` or by introducing an explicit proto3 `map` here, or alternatively, by the agent maintaining its own uuid which changes whenever a RP uuid changes and using that uuid instead of a vector here (we discussed this offline).

I'll go with making `resource_provider_id` optional. proto3 map does not support a message type key.


- Jie


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


On Oct. 19, 2017, 9:35 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 63094: Added resource version uuid for offer operations.

Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review188829
-----------------------------------------------------------




src/messages/messages.proto
Lines 613 (patched)
<https://reviews.apache.org/r/63094/#comment265828>

    See below.



src/messages/messages.proto
Lines 663 (patched)
<https://reviews.apache.org/r/63094/#comment265827>

    Even though currently operation on agent resources cannot exhibit operation speculation failures, I would still prefer would we include some version uuid for the agent resources here as well.
    
    AFAICT this could happen either by adjusting `ResourceVersionUUID` so that `resource_provider_id` becomes `optional` or by introducing an explicit proto3 `map` here, or alternatively, by the agent maintaining its own uuid which changes whenever a RP uuid changes and using that uuid instead of a vector here (we discussed this offline).



src/messages/messages.proto
Lines 712 (patched)
<https://reviews.apache.org/r/63094/#comment265829>

    See comment on `UpdateSlaveMessage`.


- Benjamin Bannier


On Oct. 19, 2017, 11:35 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2017, 11:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 63094: Added resource version uuid for offer operations.

Posted by Jie Yu <yu...@gmail.com>.

> On Oct. 26, 2017, 11:08 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 712 (patched)
> > <https://reviews.apache.org/r/63094/diff/3/?file=1864433#file1864433line712>
> >
> >     Do we need multiple ResourceVersionUUIDs here? Can the master just include the UUID for the relevant resource provider/agent?

Yeah, good point.


- Jie


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


On Oct. 19, 2017, 9:35 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 63094: Added resource version uuid for offer operations.

Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63094/#review189371
-----------------------------------------------------------




include/mesos/v1/resource_provider/resource_provider.proto
Lines 116 (patched)
<https://reviews.apache.org/r/63094/#comment266414>

    s/The resource version UUID is used/Used/
    
    Here and elsewhere.



src/messages/messages.proto
Lines 608 (patched)
<https://reviews.apache.org/r/63094/#comment266416>

    s/Describe/Describes/



src/messages/messages.proto
Lines 712 (patched)
<https://reviews.apache.org/r/63094/#comment266410>

    Do we need multiple ResourceVersionUUIDs here? Can the master just include the UUID for the relevant resource provider/agent?


- Greg Mann


On Oct. 19, 2017, 9:35 p.m., Jie Yu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63094/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2017, 9:35 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The resource version UUID is used to establish the relationship
> between the operation and the resources that the operation is
> operating on. Each resource provider will keep a resource version
> UUID, and changes it when it believes that the resources from this
> resource provider are out of sync from the master's view.  The master
> will keep track of the last known resource version UUID for each
> resource provider, and attach the resource version UUID in each
> operation it sends out. The resource provider should reject operations
> that have a different resource version UUID than that it maintains,
> because this means the operation is operating on resources that might
> have already been invalidated.
> 
> 
> Diffs
> -----
> 
>   include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
>   include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
>   src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
>   src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
>   src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 
> 
> 
> Diff: https://reviews.apache.org/r/63094/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Jie Yu
> 
>


Re: Review Request 63094: Added resource version uuid for offer operations.

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

(Updated Oct. 19, 2017, 9:35 p.m.)


Review request for mesos, Benjamin Bannier, Benjamin Mahler, Chun-Hung Hsiao, Gaston Kleiman, Greg Mann, Joseph Wu, Jan Schlicht, and Vinod Kone.


Repository: mesos


Description
-------

The resource version UUID is used to establish the relationship
between the operation and the resources that the operation is
operating on. Each resource provider will keep a resource version
UUID, and changes it when it believes that the resources from this
resource provider are out of sync from the master's view.  The master
will keep track of the last known resource version UUID for each
resource provider, and attach the resource version UUID in each
operation it sends out. The resource provider should reject operations
that have a different resource version UUID than that it maintains,
because this means the operation is operating on resources that might
have already been invalidated.


Diffs (updated)
-----

  include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845 
  include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c 
  src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f 
  src/tests/resource_provider_manager_tests.cpp ca49e1f0203494fc8b4a4507c33e5a3885a14a59 
  src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c 


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

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


Testing
-------

make check


Thanks,

Jie Yu