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/15 04:30:31 UTC
Review Request 63001: Updated protobuf definitions related to offer
operations.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/1/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer 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/63001/#review188081
-----------------------------------------------------------
PASS: Mesos patch 63001 was successfully built and tested.
Reviews applied: `['62974', '63001']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001
- Mesos Reviewbot Windows
On Oct. 15, 2017, 4:30 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2017, 4:30 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Benjamin Bannier <bb...@apache.org>.
> On Oct. 16, 2017, 12:56 p.m., Jan Schlicht wrote:
> > src/messages/messages.proto
> > Lines 643-648 (patched)
> > <https://reviews.apache.org/r/63001/diff/1/?file=1856406#file1856406line643>
> >
> > For certain operations like `CREATE_VOLUME` the resources that resulted from applying the operation have to be sent here as part of this operation. Otherwise the master won't be able to recover these resources and offer them to frameworks.
> > Also the `operation` (or at least its resources) need to be included to be able to rollback resources in case of a failure.
+1 to what Jan wrote. Would it make sense to send both the `source` and `target` resources as part of the feedback? That would enable a pretty streamlined handling, and avoid introducing into the master the current asymmetry in how operations are specified by users (e.g., `RESERVE` will send `target` while e.g., `CREATE_VOLUME` will send `source`).
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188116
-----------------------------------------------------------
On Oct. 15, 2017, 6:30 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2017, 6:30 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 10:56 a.m., Jan Schlicht wrote:
> > src/messages/messages.proto
> > Lines 643-648 (patched)
> > <https://reviews.apache.org/r/63001/diff/1/?file=1856406#file1856406line643>
> >
> > For certain operations like `CREATE_VOLUME` the resources that resulted from applying the operation have to be sent here as part of this operation. Otherwise the master won't be able to recover these resources and offer them to frameworks.
> > Also the `operation` (or at least its resources) need to be included to be able to rollback resources in case of a failure.
>
> Benjamin Bannier wrote:
> +1 to what Jan wrote. Would it make sense to send both the `source` and `target` resources as part of the feedback? That would enable a pretty streamlined handling, and avoid introducing into the master the current asymmetry in how operations are specified by users (e.g., `RESERVE` will send `target` while e.g., `CREATE_VOLUME` will send `source`).
Yup, I'll add `convered_resources` to `OfferOperationStatus`. It only applies to `OPERATION_FINISHED`
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188116
-----------------------------------------------------------
On Oct. 15, 2017, 4:30 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2017, 4:30 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188116
-----------------------------------------------------------
include/mesos/v1/resource_provider/resource_provider.proto
Lines 89-91 (original), 83-87 (patched)
<https://reviews.apache.org/r/63001/#comment265164>
It's important to include the result of certain operations, i.e., `converted_resources`. `converted_resources` provides the result of RP operations like `CREATE_VOLUME`.
Also the `operation` (or at least its resources) need to be included to be able to rollback resources in case of a failure.
src/messages/messages.proto
Lines 643-648 (patched)
<https://reviews.apache.org/r/63001/#comment265163>
For certain operations like `CREATE_VOLUME` the resources that resulted from applying the operation have to be sent here as part of this operation. Otherwise the master won't be able to recover these resources and offer them to frameworks.
Also the `operation` (or at least its resources) need to be included to be able to rollback resources in case of a failure.
- Jan Schlicht
On Oct. 15, 2017, 6:30 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2017, 6:30 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188213
-----------------------------------------------------------
include/mesos/resource_provider/resource_provider.proto
Lines 91 (patched)
<https://reviews.apache.org/r/63001/#comment265232>
This should be of type `OfferOperationStatus`, so that it includes the `converted_resources`, which the master will need if it is a terminal state.
- Gaston Kleiman
On Oct. 16, 2017, 11:32 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 11:32 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 7:09 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 665 (patched)
> > <https://reviews.apache.org/r/63001/diff/3/?file=1857695#file1857695line665>
> >
> > Do we want to follow the `_info` naming scheme even though the message isn't called `OperationInfo`?
`Offer.Operation` should really be called `OfferOperationInfo` :)
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188194
-----------------------------------------------------------
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188194
-----------------------------------------------------------
src/messages/messages.proto
Lines 665 (patched)
<https://reviews.apache.org/r/63001/#comment265226>
Do we want to follow the `_info` naming scheme even though the message isn't called `OperationInfo`?
- Greg Mann
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer 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/63001/#review188246
-----------------------------------------------------------
PASS: Mesos patch 63001 was successfully built and tested.
Reviews applied: `['62974', '63001']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001
- Mesos Reviewbot Windows
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188243
-----------------------------------------------------------
Patch looks great!
Reviews applied: [62974, 63001]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 17, 2017, 3:45 p.m., Jan Schlicht wrote:
> > include/mesos/mesos.proto
> > Lines 2187-2195 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859672#file1859672line2187>
> >
> > Is the master supposed to keep a `map<OfferOperationID, Offer::Operation>`? If not, we'll need the the resources of the operation here, to be able to rollback in case of an error.
Yes, master will keep track of all known pending/terminal but unacked operations.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188286
-----------------------------------------------------------
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jan Schlicht <ja...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188286
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 2187-2195 (patched)
<https://reviews.apache.org/r/63001/#comment265312>
Is the master supposed to keep a `map<OfferOperationID, Offer::Operation>`? If not, we'll need the the resources of the operation here, to be able to rollback in case of an error.
include/mesos/resource_provider/resource_provider.proto
Line 66 (original), 72 (patched)
<https://reviews.apache.org/r/63001/#comment265282>
Change the comment to `// See 'UpdateOperationStatus' below.`
include/mesos/v1/resource_provider/resource_provider.proto
Line 66 (original), 72 (patched)
<https://reviews.apache.org/r/63001/#comment265283>
Change the comment to `// See 'UpdateOperationStatus' below.`
src/messages/messages.proto
Lines 637-638 (patched)
<https://reviews.apache.org/r/63001/#comment265313>
Please correct the spacing here.
- Jan Schlicht
On Oct. 17, 2017, 7:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 7:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188366
-----------------------------------------------------------
src/resource_provider/validation.cpp
Line 59 (original), 59 (patched)
<https://reviews.apache.org/r/63001/#comment265353>
s/update/update_operation_status/
src/tests/resource_provider_validation_tests.cpp
Line 57 (original), 57 (patched)
<https://reviews.apache.org/r/63001/#comment265356>
s/Update/UpdateOperationStatus/
src/tests/resource_provider_validation_tests.cpp
Line 62 (original), 62 (patched)
<https://reviews.apache.org/r/63001/#comment265357>
Ditto.
src/tests/resource_provider_validation_tests.cpp
Line 69 (original), 69 (patched)
<https://reviews.apache.org/r/63001/#comment265358>
Ditto.
- Chun-Hung Hsiao
On Oct. 17, 2017, 6:52 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 6:52 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 18, 2017, 4:04 a.m., Chun-Hung Hsiao wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Lines 47-48 (original), 47-48 (patched)
> > <https://reviews.apache.org/r/63001/diff/8/?file=1861933#file1861933line47>
> >
> > Do we need to expose `framework_id` to RP if we have `operation_uuid`?
We need to. The `OfferOperation` that the RP needs to report back to RP manager after re-registration need `framework_id`.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188451
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188451
-----------------------------------------------------------
include/mesos/resource_provider/resource_provider.proto
Lines 47-48 (original), 47-48 (patched)
<https://reviews.apache.org/r/63001/#comment265447>
Do we need to expose `framework_id` to RP if we have `operation_uuid`?
include/mesos/resource_provider/resource_provider.proto
Line 95 (original), 94 (patched)
<https://reviews.apache.org/r/63001/#comment265448>
Ditto.
- Chun-Hung Hsiao
On Oct. 17, 2017, 10:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/8/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189478
-----------------------------------------------------------
src/messages/messages.proto
Lines 649 (patched)
<https://reviews.apache.org/r/63001/#comment266572>
s/UPDATE_OPERATION_STATUS/UPDATE_OFFER_OPERATION_STATUS/
- Chun-Hung Hsiao
On Oct. 27, 2017, 12:58 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2017, 12:58 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/10/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189460
-----------------------------------------------------------
Fix it, then Ship it!
include/mesos/resource_provider/resource_provider.proto
Lines 52 (patched)
<https://reviews.apache.org/r/63001/#comment266560>
s/framework specified/framework-specified/
s/id/ID/
include/mesos/resource_provider/resource_provider.proto
Lines 99 (patched)
<https://reviews.apache.org/r/63001/#comment266561>
s/framework specified/framework-specified/
s/id/ID/
src/messages/messages.proto
Line 610 (original), 610 (patched)
<https://reviews.apache.org/r/63001/#comment266559>
s/RESORUCE/RESOURCE/
src/messages/messages.proto
Lines 658 (patched)
<https://reviews.apache.org/r/63001/#comment266562>
s/framework specified/framework-specified/
s/id/ID/
src/messages/messages.proto
Lines 675 (patched)
<https://reviews.apache.org/r/63001/#comment266563>
s/framework specified/framework-specified/
s/id/ID/
- Greg Mann
On Oct. 27, 2017, 9:04 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2017, 9:04 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/11/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 29, 2017, 3:13 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, 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
-------
Updated protobuf definitions related to offer operations.
Diffs
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/12/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 29, 2017, 3:12 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Bugs: MESOS-7235
https://issues.apache.org/jira/browse/MESOS-7235
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/12/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 29, 2017, 2:36 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed comments.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/12/
Changes: https://reviews.apache.org/r/63001/diff/11-12/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 27, 2017, 9:04 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed comments from Chun.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/11/
Changes: https://reviews.apache.org/r/63001/diff/10-11/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 27, 2017, 12:58 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed Greg's comments.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/10/
Changes: https://reviews.apache.org/r/63001/diff/9-10/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 19, 2017, 11:50 a.m., Benjamin Bannier wrote:
> > include/mesos/mesos.proto
> > Lines 2199-2201 (patched)
> > <https://reviews.apache.org/r/63001/diff/9/?file=1863088#file1863088line2199>
> >
> > We will also need to model the case where an offer operation succeeds only partially (think of a `RESERVE` over multiple providers where it fail on just one).
> >
> > What should `converted_resources` be in such a case? Should it include the "unconverted" part as well?
Each offer operation should only operate on resources from one provider. I'll be a validation error if it is not.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188668
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Benjamin Bannier <bb...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188668
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 2199-2201 (patched)
<https://reviews.apache.org/r/63001/#comment265686>
We will also need to model the case where an offer operation succeeds only partially (think of a `RESERVE` over multiple providers where it fail on just one).
What should `converted_resources` be in such a case? Should it include the "unconverted" part as well?
- Benjamin Bannier
On Oct. 19, 2017, 2:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 2:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 23, 2017, 11:33 p.m., Gaston Kleiman wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2136 (patched)
> > <https://reviews.apache.org/r/63001/diff/9/?file=1863090#file1863090line2136>
> >
> > I think that "info" is a bit too generic. What about "operation_info" or "operation"?
The message has type `OfferOperation`, meaning that the context is clear: it is for an offer operation. It reads well like the following
```
OfferOperation operation;
xxx = operation.info().yyy;
```
compared to
```
OfferOperation operation;
xxx = operation.operation_info().yyy;
```
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189004
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189004
-----------------------------------------------------------
include/mesos/v1/mesos.proto
Lines 2136 (patched)
<https://reviews.apache.org/r/63001/#comment265933>
I think that "info" is a bit too generic. What about "operation_info" or "operation"?
- Gaston Kleiman
On Oct. 18, 2017, 5:08 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 18, 2017, 5:08 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 26, 2017, 10:49 p.m., Greg Mann wrote:
> > include/mesos/resource_provider/resource_provider.proto
> > Line 66 (original), 72 (patched)
> > <https://reviews.apache.org/r/63001/diff/9/?file=1863089#file1863089line72>
> >
> > Do you want to use `OFFER_OPERATION` here instead? Or, do you think it's OK to just use `OPERATION` since this is within the RP? Here and elsewhere.
Yeah, I'll use OFFER_OPERATION consistently.
> On Oct. 26, 2017, 10:49 p.m., Greg Mann wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2145 (patched)
> > <https://reviews.apache.org/r/63001/diff/9/?file=1863090#file1863090line2145>
> >
> > I'm a little bit concerned that the names of the various IDs in this RR will become confusing for devs. We have:
> > 1) `OfferOperationID`, with a field name of `operation_id`
> > 2) `bytes uuid`, with a field name of `uuid` (this is the status update ID)
> > 3) `bytes uuid`, with a field name of `operation_uuid` (this is our internal ID for the operation)
> >
> > What would you think about naming the fields `operation_id`, `operation_update_uuid`, and `operation_internal_uuid`, respectively? (could also just do `update_uuid` and `internal_uuid` for the latter two)
> >
> > Since the fields are in different messages, I'm not sure how bad this would really be, just a thought. Let me know what you think.
In `Offer.Operation`, the field is `id`, which reads well because the context is clear. In `OfferOperationStatus`, it's `operation_id`, which also makes sense to me because we might have different `id` in that message.
In `OfferOperation`, it's `operation_uuid`. The reason for `operation_` prefix is because there are `framework_id` in the message. Adding a prefix makes the context more clear (similar to above). So each offer operation has a `id` (specified by the framework) and a `uuid` which is generated by the master.
I agree with you that `uuid` in `OfferOperationStatus` is a bit confusing. How about renaming it to `status_uuid`?
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189354
-----------------------------------------------------------
On Oct. 27, 2017, 12:58 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 27, 2017, 12:58 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/10/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review189354
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 2157 (patched)
<https://reviews.apache.org/r/63001/#comment266369>
s/status/statuses/
include/mesos/mesos.proto
Lines 2157-2158 (patched)
<https://reviews.apache.org/r/63001/#comment266370>
Can we clarify here that this is an ordered list?
include/mesos/mesos.proto
Lines 2162 (patched)
<https://reviews.apache.org/r/63001/#comment266372>
s/framework specified/framework-specified/
s/id/ID/ ??
Here and below.
include/mesos/mesos.proto
Lines 2204 (patched)
<https://reviews.apache.org/r/63001/#comment266394>
Backticks instead of quotes here for consistency.
include/mesos/resource_provider/resource_provider.proto
Line 66 (original), 72 (patched)
<https://reviews.apache.org/r/63001/#comment266396>
Do you want to use `OFFER_OPERATION` here instead? Or, do you think it's OK to just use `OPERATION` since this is within the RP? Here and elsewhere.
include/mesos/v1/mesos.proto
Lines 2145 (patched)
<https://reviews.apache.org/r/63001/#comment266392>
I'm a little bit concerned that the names of the various IDs in this RR will become confusing for devs. We have:
1) `OfferOperationID`, with a field name of `operation_id`
2) `bytes uuid`, with a field name of `uuid` (this is the status update ID)
3) `bytes uuid`, with a field name of `operation_uuid` (this is our internal ID for the operation)
What would you think about naming the fields `operation_id`, `operation_update_uuid`, and `operation_internal_uuid`, respectively? (could also just do `update_uuid` and `internal_uuid` for the latter two)
Since the fields are in different messages, I'm not sure how bad this would really be, just a thought. Let me know what you think.
include/mesos/v1/mesos.proto
Lines 2185 (patched)
<https://reviews.apache.org/r/63001/#comment266393>
Backticks instead of quotes here for consistency.
src/messages/messages.proto
Lines 608-610 (original), 608-610 (patched)
<https://reviews.apache.org/r/63001/#comment266402>
Add a note about offer operations here?
src/messages/messages.proto
Lines 638 (patched)
<https://reviews.apache.org/r/63001/#comment266401>
s/update/updates/
- Greg Mann
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 19, 2017, 12:08 a.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Rebased.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/9/
Changes: https://reviews.apache.org/r/63001/diff/8-9/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Oct. 18, 2017, 8:21 p.m., Chun-Hung Hsiao wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2170 (patched)
> > <https://reviews.apache.org/r/63001/diff/8/?file=1861934#file1861934line2170>
> >
> > This protobuf is exposed to RP API. Do we want an RP to be aware of the scheduler? Do we allow each RP to decide if an operation should be acked? If we don't, then I think we should handle this `uuid` in the RP manager. If we do, then we should also include the ACK mechanism in the RP API.
Since the RP is responsible for status update retries, I think that the RP needs to know this 'uuid' in order to confirm status update acknowledgements that it receives from the framework.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188565
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188565
-----------------------------------------------------------
Since we're designing the RP API, I'd prefer to only expose fields that are required by RP's polymorphic behaviors, and hide the things that should only be known by Mesos in the RP manager, even though this implies that the code of the RP manager would become more complicated, which I think it is an affordable cost in general.
include/mesos/v1/mesos.proto
Lines 2170 (patched)
<https://reviews.apache.org/r/63001/#comment265566>
This protobuf is exposed to RP API. Do we want an RP to be aware of the scheduler? Do we allow each RP to decide if an operation should be acked? If we don't, then I think we should handle this `uuid` in the RP manager. If we do, then we should also include the ACK mechanism in the RP API.
- Chun-Hung Hsiao
On Oct. 17, 2017, 10:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/8/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 10:10 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed Greg's comment.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/8/
Changes: https://reviews.apache.org/r/63001/diff/7-8/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 17, 2017, 10:02 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 2178 (patched)
> > <https://reviews.apache.org/r/63001/diff/7/?file=1861113#file1861113line2178>
> >
> > Let's make this `optional` to handle cases where we send `OfferOperationStatus` messages internally for operations which the scheduler does not want feedback about.
Good catch!
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188403
-----------------------------------------------------------
On Oct. 17, 2017, 8:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 8:10 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/7/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188403
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 2178 (patched)
<https://reviews.apache.org/r/63001/#comment265396>
Let's make this `optional` to handle cases where we send `OfferOperationStatus` messages internally for operations which the scheduler does not want feedback about.
- Greg Mann
On Oct. 17, 2017, 8:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 8:10 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/7/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 8:10 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed comments from Chun
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/7/
Changes: https://reviews.apache.org/r/63001/diff/6-7/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 6:52 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Fixed a typo.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/6/
Changes: https://reviews.apache.org/r/63001/diff/5-6/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 6:49 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/5/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 6:49 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed comments. Rebased.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/5/
Changes: https://reviews.apache.org/r/63001/diff/4-5/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Chun-Hung Hsiao <ch...@mesosphere.io>.
> On Oct. 17, 2017, 5:51 p.m., Greg Mann wrote:
> > include/mesos/mesos.proto
> > Lines 2187-2195 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859672#file1859672line2187>
> >
> > We also need to include a unique UUID for each operation status update, so that the framework can include it when acknowledging updates.
> >
> > If we follow the pattern of task status updates, we could include an `optional bytes uuid` field here.
Hmm I missed the comment here :(. Please see my comments below. How about a wrapper protobuf that includes an `OfferOperationStatus` and a `optional bytes uuid` that is only used internally in Mesos, or introduce another protobuf for `resource_provider.Call.UpdateOperationStatus` instead of reusing this one?
- Chun-Hung
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188340
-----------------------------------------------------------
On Oct. 17, 2017, 10:10 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 10:10 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/8/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188340
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 2187-2195 (patched)
<https://reviews.apache.org/r/63001/#comment265329>
We also need to include a unique UUID for each operation status update, so that the framework can include it when acknowledging updates.
If we follow the pattern of task status updates, we could include an `optional bytes uuid` field here.
- Greg Mann
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 17, 2017, 5:59 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 633-639 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859676#file1859676line633>
> >
> > I think we also need to include this field in the `ReregisterSlaveMessage`, right? That would accommodate RPs which have already registered with the agent when the agent registers with the master.
In fact, in `ReregisterSlaveMessage`, we don't inform the master about the resources that are provided by any of the LRP. Currently, we rely on the subsequent `UpdateSlaveMessage` to do that.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188350
-----------------------------------------------------------
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Oct. 17, 2017, 5:59 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 633-639 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859676#file1859676line633>
> >
> > I think we also need to include this field in the `ReregisterSlaveMessage`, right? That would accommodate RPs which have already registered with the agent when the agent registers with the master.
>
> Jie Yu wrote:
> In fact, in `ReregisterSlaveMessage`, we don't inform the master about the resources that are provided by any of the LRP. Currently, we rely on the subsequent `UpdateSlaveMessage` to do that.
Ah OK, thx Jie!
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188350
-----------------------------------------------------------
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188350
-----------------------------------------------------------
src/messages/messages.proto
Lines 633-639 (patched)
<https://reviews.apache.org/r/63001/#comment265332>
I think we also need to include this field in the `ReregisterSlaveMessage`, right? That would accommodate RPs which have already registered with the agent when the agent registers with the master.
- Greg Mann
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer 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/63001/#review188280
-----------------------------------------------------------
FAIL: Some Mesos tests failed.
Reviews applied: `['62974', '63001']`
Failed command: `C:\mesos\src\mesos-tests.exe --verbose --gtest_filter="-ContentType/MasterAPITest.EventAuthorizationFiltering/1"`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001
Relevant logs:
- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001/logs/mesos-tests-stdout.log):
```
[ OK ] Endpoint/SlaveEndpointTest.UnauthorizedRequest/0 (309 ms)
[ RUN ] Endpoint/SlaveEndpointTest.UnauthorizedRequest/1
[ OK ] Endpoint/SlaveEndpointTest.UnauthorizedRequest/1 (264 ms)
[ RUN ] Endpoint/SlaveEndpointTest.UnauthorizedRequest/2
[ OK ] Endpoint/SlaveEndpointTest.UnauthorizedRequest/2 (291 ms)
[ RUN ] Endpoint/SlaveEndpointTest.NoAuthorizer/0
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/0 (285 ms)
[ RUN ] Endpoint/SlaveEndpointTest.NoAuthorizer/1
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/1 (305 ms)
[ RUN ] Endpoint/SlaveEndpointTest.NoAuthorizer/2
[ OK ] Endpoint/SlaveEndpointTest.NoAuthorizer/2 (305 ms)
[----------] 9 tests from Endpoint/SlaveEndpointTest (3111 ms total)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (139 ms)
[ RUN ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[ OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (144 ms)
[----------] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (358 ms total)
[----------] Global test environment tear-down
[==========] 723 tests from 73 test cases ran. (369739 ms total)
[ PASSED ] 722 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] ResourceProviderCallValidationTest.Update
1 FAILED TEST
YOU HAVE 166 DISABLED TESTS
```
- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001/logs/mesos-tests-stderr.log):
```
I1017 06:58:32.541306 62212 containerizer.cpp:649] Recovering containerizer
I1017 06:58:32.548305 57708 provisioner.cpp:416] Provisioner recovery complete
I1017 06:58:32.549304 58452 slave.cpp:6322] Finished recovery
I1017 06:58:32.574306 62212 http.cpp:1185] HTTP GET for /slave(270)/monitor/statistics.json from 10.3.1.7:65068
I1017 06:58:32.575306 62212 http.cpp:976] Authorizing principal 'test-principal' to GET the '/monitor/statistics.json' endpoint
I1017 06:58:32.598309 61284 slave.cpp:869] Agent terminating
I1017 06:58:32.822024 63224 containerizer.cpp:292] Using isolation { windows/cpu, filesystem/windows, environment_secret }
I1017 06:58:32.825024 63224 provisioner.cpp:255] Using default backend 'copy'
I1017 06:58:32.842025 63224 cluster.cpp:448] Creating default 'local' authorizer
I1017 06:58:32.861024 61284 slave.cpp:254] Mesos agent started on (271)@10.3.1.7:64131
I1017 06:58:32.862025 61284 slave.cpp:255] Flags at startup: --acls="" --appc_simple_discovery_uri_prefix="http://" --appc_store_dir="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\store\appc" --authenticate_http_readonly="true" --authenticate_http_readwrite="true" --authenticatee="crammd5" --authentication_backoff_factor="1secs" --authorizer="local" --container_disk_watch_interval="15secs" --containerizers="mesos" --credential="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\credential" --default_role="*" --disk_watch_interval="1mins" --docker="docker" --docker_kill_orphans="true" --docker_registry="https://registry-1.docker.io" --docker_remove_delay="6hrs" --docker_socket="//./pipe/docker_engine" --docker_stop_timeout="0ns" --docker_store_dir="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\store\docker" --docker_volume_checkpoint_dir="/var/run/mesos/isolators/docker/volume" --enforce_container_disk_quota="false" --executor_registration_timeout="1mins" --executor_reregistration_timeout="2secs" -
-executor_shutdown_grace_period="5secs" --fetcher_cache_dir="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\fetch" --fetcher_cache_size="2GB" --frameworks_home="" --gc_delay="1weeks" --gc_disk_headroom="0.1" --hadoop_home="" --help="false" --hostname_lookup="true" --http_command_executor="false" --http_credentials="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\http_credentials" --http_heartbeat_interval="30secs" --initialize_driver_logging="true" --isolation="windows/cpu" --launcher="windows" --launcher_dir="C:\mesos\src" --logbufsecs="0" --logging_level="INFO" --max_completed_executors_per_framework="150" --oversubscribed_resources_interval="15secs" --port="5051" --qos_correction_interval_min="0ns" --quiet="false" --recover="reconnect" --recovery_timeout="15mins" --registration_backoff_factor="10ms" --resources="cpus:2;gpus:0;mem:1024;disk:1024;ports:[31000-32000]" --runtime_dir="C:\Users\mesos\AppData\Local\Temp\2\VmAS6A" --sandbox_directory="C:\mesos\sandbox" --strict="true" --version="
false" --work_dir="C:\Users\mesos\AppData\Local\Temp\2\EAuOhF" --zk_session_timeout="10secs"
I1017 06:58:32.888038 61284 credentials.hpp:86] Loading credential for authentication from 'C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\credential'
I1017 06:58:32.889029 61284 slave.cpp:287] Agent using credential for: test-principal
I1017 06:58:32.890029 61284 credentials.hpp:37] Loading credentials for authentication from 'C:\Users\mesos\AppData\Local\Temp\2\VmAS6A\http_credentials'
I1017 06:58:32.892030 61284 http.cpp:1045] Creating default 'basic' HTTP authenticator for realm 'mesos-agent-readonly'
I1017 06:58:32.893029 61284 http.cpp:1045] Creating default 'basic' HTTP authenticator for realm 'mesos-agent-readwrite'
I1017 06:58:32.929026 61284 slave.cpp:585] Agent resources: [{"name":"cpus","scalar":{"value":2.0},"type":"SCALAR"},{"name":"mem","scalar":{"value":1024.0},"type":"SCALAR"},{"name":"disk","scalar":{"value":1024.0},"type":"SCALAR"},{"name":"ports","ranges":{"range":[{"begin":31000,"end":32000}]},"type":"RANGES"}]
I1017 06:58:32.932026 61284 slave.cpp:593] Agent attributes: [ ]
I1017 06:58:32.933027 61284 slave.cpp:602] Agent hostname: mesos-bld-s2.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I1017 06:58:32.933027 57708 status_update_manager.cpp:177] Pausing sending status updates
I1017 06:58:32.944025 58936 state.cpp:64] Recovering state from 'C:\Users\mesos\AppData\Local\Temp\2\EAuOhF\meta'
I1017 06:58:32.945026 57708 status_update_manager.cpp:203] Recovering status update manager
I1017 06:58:32.946027 62212 containerizer.cpp:649] Recovering containerizer
I1017 06:58:32.952028 61284 provisioner.cpp:416] Provisioner recovery complete
I1017 06:58:32.952028 62176 slave.cpp:6322] Finished recovery
I1017 06:58:32.988028 62176 http.cpp:1185] HTTP GET for /slave(271)/containers from 10.3.1.7:65069
I1017 06:58:32.989028 62176 http.cpp:976] Authorizing principal 'test-principal' to GET the '/containers' endpoint
I1017 06:58:33.008028 62176 slave.cpp:869] Agent terminating
I1017 06:58:34.312995 61716 process.cpp:1068] Failed to accept socket: future discarded
```
- Mesos Reviewbot Windows
On Oct. 17, 2017, 1:08 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 1:08 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 17, 2017, 1:17 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto
> > Lines 111 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859672#file1859672line111>
> >
> > unique in the context of the framework? or universally unique?
in the context of the framework. made it more clear in the comments.
> On Oct. 17, 2017, 1:17 p.m., James DeFelice wrote:
> > include/mesos/mesos.proto
> > Lines 2166 (patched)
> > <https://reviews.apache.org/r/63001/diff/4/?file=1859672#file1859672line2166>
> >
> > isn't UNKNOWN standard here?
unfortunately, `OFFER_OPERATION_UNKNOWN` has a special meaning (similar to `TASK_UNKNOWN`). Now I think about it, prefer to calling it `OFFER_OPERATION_UNSUPPORTED`
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188305
-----------------------------------------------------------
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by James DeFelice <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188305
-----------------------------------------------------------
include/mesos/mesos.proto
Lines 111 (patched)
<https://reviews.apache.org/r/63001/#comment265303>
unique in the context of the framework? or universally unique?
include/mesos/mesos.proto
Lines 2166 (patched)
<https://reviews.apache.org/r/63001/#comment265302>
isn't UNKNOWN standard here?
- James DeFelice
On Oct. 17, 2017, 5:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 5:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 17, 2017, 5:08 a.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed review comments.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/4/
Changes: https://reviews.apache.org/r/63001/diff/3-4/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > <https://reviews.apache.org/r/63001/diff/1-2/?file=1856406#file1856406line637>
> >
> > I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
> >
> > When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
> >
> > If the agent includes only pending operations in this field, then the master will be able to make this distinction.
The reason we need to send terminal but unacked operations is for reconciliation. Master always keeps a cache about all the known non-completed operations (either pending or terminal but unacked). This information can be used to answer reconciliation requests.
The master can distinguish between a pending operation vs. a terminal but unacked operation by looking at latest state of the operation. The master update the allocator if the latest state if terminal, irrespective whether it's acked or not.
That does bring up an issue. Consider the following two cases:
1) Master knows about an operation that the agent does not know about. This can happen if the operation gets lost on its way to the agent. This will trigger an agent re-registration. The master in this case will need to generate a reconcile message (similar to that in `reconcileKnownSlave`) so that the agent or LRP can reply with a terminal status update. This will guarantee that the resources allocated for this operation are properly recovered to allocator.
This case is also possible if there is a speculation failure on old operations and result in an `UpdateSlaveMessage` being sent to the master. The operation that master knows about (but agent does not) will eventually reach agent or LRP, and will be rejected by the LRP or the agent. The master will properly recovered the resources to the allocator when a failed update is received.
2) Agent knows about an operation that the master does not know about. For instance, This is a new master that just fails over, and an LRP re-registers with the agent. In that case, the master needs to update the 'allocated' (i.e., 'used') resources for those new operations. This is similar to what we do when slave registers. However, the tricky part is that the allocator does not have an interface currently allowing us to add more allocation to a given agent. We likely need to add that (or make it part of `Allocator::updateSlave` interface).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > <https://reviews.apache.org/r/63001/diff/1-2/?file=1856406#file1856406line637>
> >
> > I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
> >
> > When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
> >
> > If the agent includes only pending operations in this field, then the master will be able to make this distinction.
>
> Jie Yu wrote:
> The reason we need to send terminal but unacked operations is for reconciliation. Master always keeps a cache about all the known non-completed operations (either pending or terminal but unacked). This information can be used to answer reconciliation requests.
>
> The master can distinguish between a pending operation vs. a terminal but unacked operation by looking at latest state of the operation. The master update the allocator if the latest state if terminal, irrespective whether it's acked or not.
>
> That does bring up an issue. Consider the following two cases:
>
> 1) Master knows about an operation that the agent does not know about. This can happen if the operation gets lost on its way to the agent. This will trigger an agent re-registration. The master in this case will need to generate a reconcile message (similar to that in `reconcileKnownSlave`) so that the agent or LRP can reply with a terminal status update. This will guarantee that the resources allocated for this operation are properly recovered to allocator.
>
> This case is also possible if there is a speculation failure on old operations and result in an `UpdateSlaveMessage` being sent to the master. The operation that master knows about (but agent does not) will eventually reach agent or LRP, and will be rejected by the LRP or the agent. The master will properly recovered the resources to the allocator when a failed update is received.
>
> 2) Agent knows about an operation that the master does not know about. For instance, This is a new master that just fails over, and an LRP re-registers with the agent. In that case, the master needs to update the 'allocated' (i.e., 'used') resources for those new operations. This is similar to what we do when slave registers. However, the tricky part is that the allocator does not have an interface currently allowing us to add more allocation to a given agent. We likely need to add that (or make it part of `Allocator::updateSlave` interface).
OK, I see why we need to include unacked updates as well. Thanks Jie!
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 17, 2017, 6:52 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 17, 2017, 6:52 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > <https://reviews.apache.org/r/63001/diff/1-2/?file=1856406#file1856406line637>
> >
> > I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
> >
> > When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
> >
> > If the agent includes only pending operations in this field, then the master will be able to make this distinction.
>
> Jie Yu wrote:
> The reason we need to send terminal but unacked operations is for reconciliation. Master always keeps a cache about all the known non-completed operations (either pending or terminal but unacked). This information can be used to answer reconciliation requests.
>
> The master can distinguish between a pending operation vs. a terminal but unacked operation by looking at latest state of the operation. The master update the allocator if the latest state if terminal, irrespective whether it's acked or not.
>
> That does bring up an issue. Consider the following two cases:
>
> 1) Master knows about an operation that the agent does not know about. This can happen if the operation gets lost on its way to the agent. This will trigger an agent re-registration. The master in this case will need to generate a reconcile message (similar to that in `reconcileKnownSlave`) so that the agent or LRP can reply with a terminal status update. This will guarantee that the resources allocated for this operation are properly recovered to allocator.
>
> This case is also possible if there is a speculation failure on old operations and result in an `UpdateSlaveMessage` being sent to the master. The operation that master knows about (but agent does not) will eventually reach agent or LRP, and will be rejected by the LRP or the agent. The master will properly recovered the resources to the allocator when a failed update is received.
>
> 2) Agent knows about an operation that the master does not know about. For instance, This is a new master that just fails over, and an LRP re-registers with the agent. In that case, the master needs to update the 'allocated' (i.e., 'used') resources for those new operations. This is similar to what we do when slave registers. However, the tricky part is that the allocator does not have an interface currently allowing us to add more allocation to a given agent. We likely need to add that (or make it part of `Allocator::updateSlave` interface).
>
> Greg Mann wrote:
> OK, I see why we need to include unacked updates as well. Thanks Jie!
>
> Greg Mann wrote:
> Hey Jie, I have another question related to this issue so I re-opened it for discussion.
>
> Previously you wrote "The master update the allocator if the latest state if terminal, irrespective whether it's acked or not."
> This implies that subsequent retries of the update will _not_ have the `latest_state` set; otherwise, the master would not be able to distinguish between the first terminal status update and subsequent retries. Was it your intention to suggest that behavior? Looking at the `Slave::forward` helper that we use when intitializing the status update manager, it looks like we _always_ set `latest_state` for task status updates: https://github.com/apache/mesos/blob/f26ffcee0a359a644968feca1ec91243401f589a/src/slave/slave.cpp#L4867-L4870
>
> I suppose we could set the latest state the first time an operation update is sent to master, and then _not_ set it in subsequent retries. Then the value of `.has_latest_state()` would be a direct indication of whether or not an update is a retry.
Ah, I see that `latest_status` is optional, so I suspect this was indeed your intention. Just want to confirm :)
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > <https://reviews.apache.org/r/63001/diff/1-2/?file=1856406#file1856406line637>
> >
> > I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
> >
> > When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
> >
> > If the agent includes only pending operations in this field, then the master will be able to make this distinction.
>
> Jie Yu wrote:
> The reason we need to send terminal but unacked operations is for reconciliation. Master always keeps a cache about all the known non-completed operations (either pending or terminal but unacked). This information can be used to answer reconciliation requests.
>
> The master can distinguish between a pending operation vs. a terminal but unacked operation by looking at latest state of the operation. The master update the allocator if the latest state if terminal, irrespective whether it's acked or not.
>
> That does bring up an issue. Consider the following two cases:
>
> 1) Master knows about an operation that the agent does not know about. This can happen if the operation gets lost on its way to the agent. This will trigger an agent re-registration. The master in this case will need to generate a reconcile message (similar to that in `reconcileKnownSlave`) so that the agent or LRP can reply with a terminal status update. This will guarantee that the resources allocated for this operation are properly recovered to allocator.
>
> This case is also possible if there is a speculation failure on old operations and result in an `UpdateSlaveMessage` being sent to the master. The operation that master knows about (but agent does not) will eventually reach agent or LRP, and will be rejected by the LRP or the agent. The master will properly recovered the resources to the allocator when a failed update is received.
>
> 2) Agent knows about an operation that the master does not know about. For instance, This is a new master that just fails over, and an LRP re-registers with the agent. In that case, the master needs to update the 'allocated' (i.e., 'used') resources for those new operations. This is similar to what we do when slave registers. However, the tricky part is that the allocator does not have an interface currently allowing us to add more allocation to a given agent. We likely need to add that (or make it part of `Allocator::updateSlave` interface).
>
> Greg Mann wrote:
> OK, I see why we need to include unacked updates as well. Thanks Jie!
Hey Jie, I have another question related to this issue so I re-opened it for discussion.
Previously you wrote "The master update the allocator if the latest state if terminal, irrespective whether it's acked or not."
This implies that subsequent retries of the update will _not_ have the `latest_state` set; otherwise, the master would not be able to distinguish between the first terminal status update and subsequent retries. Was it your intention to suggest that behavior? Looking at the `Slave::forward` helper that we use when intitializing the status update manager, it looks like we _always_ set `latest_state` for task status updates: https://github.com/apache/mesos/blob/f26ffcee0a359a644968feca1ec91243401f589a/src/slave/slave.cpp#L4867-L4870
I suppose we could set the latest state the first time an operation update is sent to master, and then _not_ set it in subsequent retries. Then the value of `.has_latest_state()` would be a direct indication of whether or not an update is a retry.
- Greg
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 643-648 (patched)
> > <https://reviews.apache.org/r/63001/diff/1/?file=1856406#file1856406line643>
> >
> > I think that we need to include the agent’s clock value in this message. When the master receives an operation status update, it must be able to determine whether that update was sent before or after the next UpdateSlaveMessage from the agent. To do this, it must inspect a clock value contained in each update.
We'll introduce the clock in a separate review.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 6:33 p.m., Greg Mann wrote:
> > src/messages/messages.proto
> > Lines 637-638 (patched)
> > <https://reviews.apache.org/r/63001/diff/1-2/?file=1856406#file1856406line637>
> >
> > I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
> >
> > When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
> >
> > If the agent includes only pending operations in this field, then the master will be able to make this distinction.
>
> Jie Yu wrote:
> The reason we need to send terminal but unacked operations is for reconciliation. Master always keeps a cache about all the known non-completed operations (either pending or terminal but unacked). This information can be used to answer reconciliation requests.
>
> The master can distinguish between a pending operation vs. a terminal but unacked operation by looking at latest state of the operation. The master update the allocator if the latest state if terminal, irrespective whether it's acked or not.
>
> That does bring up an issue. Consider the following two cases:
>
> 1) Master knows about an operation that the agent does not know about. This can happen if the operation gets lost on its way to the agent. This will trigger an agent re-registration. The master in this case will need to generate a reconcile message (similar to that in `reconcileKnownSlave`) so that the agent or LRP can reply with a terminal status update. This will guarantee that the resources allocated for this operation are properly recovered to allocator.
>
> This case is also possible if there is a speculation failure on old operations and result in an `UpdateSlaveMessage` being sent to the master. The operation that master knows about (but agent does not) will eventually reach agent or LRP, and will be rejected by the LRP or the agent. The master will properly recovered the resources to the allocator when a failed update is received.
>
> 2) Agent knows about an operation that the master does not know about. For instance, This is a new master that just fails over, and an LRP re-registers with the agent. In that case, the master needs to update the 'allocated' (i.e., 'used') resources for those new operations. This is similar to what we do when slave registers. However, the tricky part is that the allocator does not have an interface currently allowing us to add more allocation to a given agent. We likely need to add that (or make it part of `Allocator::updateSlave` interface).
>
> Greg Mann wrote:
> OK, I see why we need to include unacked updates as well. Thanks Jie!
>
> Greg Mann wrote:
> Hey Jie, I have another question related to this issue so I re-opened it for discussion.
>
> Previously you wrote "The master update the allocator if the latest state if terminal, irrespective whether it's acked or not."
> This implies that subsequent retries of the update will _not_ have the `latest_state` set; otherwise, the master would not be able to distinguish between the first terminal status update and subsequent retries. Was it your intention to suggest that behavior? Looking at the `Slave::forward` helper that we use when intitializing the status update manager, it looks like we _always_ set `latest_state` for task status updates: https://github.com/apache/mesos/blob/f26ffcee0a359a644968feca1ec91243401f589a/src/slave/slave.cpp#L4867-L4870
>
> I suppose we could set the latest state the first time an operation update is sent to master, and then _not_ set it in subsequent retries. Then the value of `.has_latest_state()` would be a direct indication of whether or not an update is a retry.
>
> Greg Mann wrote:
> Ah, I see that `latest_status` is optional, so I suspect this was indeed your intention. Just want to confirm :)
Greg, the allocator will be updated only if the state is transitioning from non terminal to terminal:
https://github.com/apache/mesos/blob/master/src/master/master.cpp#L9122
No need to strip latest_state when sending the update.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
On Oct. 19, 2017, 12:08 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 19, 2017, 12:08 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 859fdff4d9a0604bc506b08af79075084ae23466
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto cfd4abd3af1d8c9fbd31659161eada9ec9f92282
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/9/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188162
-----------------------------------------------------------
include/mesos/v1/mesos.proto
Lines 2124 (patched)
<https://reviews.apache.org/r/63001/#comment265208>
s/informations/information/
src/messages/messages.proto
Lines 637-638 (patched)
<https://reviews.apache.org/r/63001/#comment265218>
I think that this list should include pending operations, but should _not_ include terminal operations with unacknowledged updates.
When the master receives an update, it needs to decide whether or not it should update the allocator. If the operation was pending when `UpdateSlaveMessage` was sent, then the master should update the allocator. If the operation was not pending but its update had not been acknowledged, then the result of that operation was already reflected in the last `UpdateSlaveMessage` from the agent, and the master should not update the allocator.
If the agent includes only pending operations in this field, then the master will be able to make this distinction.
src/messages/messages.proto
Lines 643-648 (patched)
<https://reviews.apache.org/r/63001/#comment265202>
I think that we need to include the agent’s clock value in this message. When the master receives an operation status update, it must be able to determine whether that update was sent before or after the next UpdateSlaveMessage from the agent. To do this, it must inspect a clock value contained in each update.
- Greg Mann
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer 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/63001/#review188235
-----------------------------------------------------------
PASS: Mesos patch 63001 was successfully built and tested.
Reviews applied: `['62974', '63001']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/63001
- Mesos Reviewbot Windows
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
> On Oct. 16, 2017, 9:31 p.m., Gaston Kleiman wrote:
> > include/mesos/v1/mesos.proto
> > Lines 2129 (patched)
> > <https://reviews.apache.org/r/63001/diff/3/?file=1857693#file1857693line2129>
> >
> > should this be a state or status?
I remove the `state` because it can be derived from `status` by looking at the last entry in the `statuses` list.
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188215
-----------------------------------------------------------
On Oct. 16, 2017, 6:32 p.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 6:32 p.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188215
-----------------------------------------------------------
include/mesos/v1/mesos.proto
Lines 2129 (patched)
<https://reviews.apache.org/r/63001/#comment265233>
should this be a state or status?
- Gaston Kleiman
On Oct. 16, 2017, 11:32 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2017, 11:32 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 16, 2017, 6:32 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/3/
Changes: https://reviews.apache.org/r/63001/diff/2-3/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/
-----------------------------------------------------------
(Updated Oct. 16, 2017, 6:16 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
Changes
-------
Addressed review comments.
Repository: mesos
Description
-------
Updated protobuf definitions related to offer operations.
Diffs (updated)
-----
include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
Diff: https://reviews.apache.org/r/63001/diff/2/
Changes: https://reviews.apache.org/r/63001/diff/1-2/
Testing
-------
make check
Thanks,
Jie Yu
Re: Review Request 63001: Updated protobuf definitions related to
offer operations.
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/63001/#review188147
-----------------------------------------------------------
src/messages/messages.proto
Lines 633 (patched)
<https://reviews.apache.org/r/63001/#comment265191>
We cannot disginguish:
1) not having offer_operations
2) offer_operations is empty
- Jie Yu
On Oct. 15, 2017, 4:30 a.m., Jie Yu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/63001/
> -----------------------------------------------------------
>
> (Updated Oct. 15, 2017, 4:30 a.m.)
>
>
> Review request for mesos, Benjamin Bannier, Benjamin Mahler, Gaston Kleiman, Greg Mann, Jan Schlicht, and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated protobuf definitions related to offer operations.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto ba87339dbe341f4d16ceea74adc09647a3c07f32
> include/mesos/resource_provider/resource_provider.proto f5a9073075327019fd133bd51265f695ef464845
> include/mesos/v1/mesos.proto a6d662fb26aa4f78ef20ffe6e013f7a45f7f8c21
> include/mesos/v1/resource_provider/resource_provider.proto e5cbede5b6e57a8641fca1ebfee5454f292cc24c
> src/messages/messages.proto 0a32b3457e9143a7d48670610ca3e56dd516136f
> src/resource_provider/manager.cpp 31fcb789f5ab907511e868c374c49f7457a33ed3
> src/resource_provider/validation.cpp d2927227f60ab0d4ae2481ad73a31ee444b48ee0
> src/tests/resource_provider_validation_tests.cpp f182bff4670318e9de22c2915c5dbb423a74ad6c
>
>
> Diff: https://reviews.apache.org/r/63001/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Jie Yu
>
>