Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Neil Conway]

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


Fix it, then Ship it!





include/mesos/mesos.proto
Lines 349 (patched)


"The framework is offered resources in a new format."



include/mesos/mesos.proto
Lines 359 (patched)


"The framework can create refined reservations."



include/mesos/mesos.proto
Lines 368 (patched)


Typo: "capability"

Tweak: "A resource is said to have"



include/mesos/mesos.proto
Lines 980 (patched)


Is this consistent with the "set both old and new fields for endpoints" 
approach?



include/mesos/mesos.proto
Lines 1005 (patched)


"a framework or agent"? Seems technically correct but maybe confusing to 
mention here.



include/mesos/mesos.proto
Lines 1101 (patched)


Can we add a note here that the contents of the `reservations` field are 
ordered from "least refined" to "most refined"?



include/mesos/v1/mesos.proto
Lines 367 (patched)


Typo



include/mesos/v1/mesos.proto
Lines 1151 (patched)


Remove comma: "reservation's role,"


- Neil Conway


On June 16, 2017, 10:51 p.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 16, 2017, 10:51 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> \`Resource.reservations\` rather than with \`Resource.role\` and
> \`Resource.reservation\`. We also keep track of the type of reservation,
> and the role the reservation is for in the \`Resource.ReservationInfo\`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/12/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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

(Updated June 16, 2017, 3:51 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added a few references.


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


Repository: mesos


Description
---

With reservation refinement, we now represent the reservation state in
\`Resource.reservations\` rather than with \`Resource.role\` and
\`Resource.reservation\`. We also keep track of the type of reservation,
and the role the reservation is for in the \`Resource.ReservationInfo\`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/12/

Changes: https://reviews.apache.org/r/59861/diff/11-12/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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

(Updated June 16, 2017, 3:29 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added a more descriptive comment for agent `RESERVATION_REFINEMNET` capability.


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


Repository: mesos


Description (updated)
---

With reservation refinement, we now represent the reservation state in
\`Resource.reservations\` rather than with \`Resource.role\` and
\`Resource.reservation\`. We also keep track of the type of reservation,
and the role the reservation is for in the \`Resource.ReservationInfo\`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/11/

Changes: https://reviews.apache.org/r/59861/diff/10-11/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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

(Updated June 16, 2017, 2:43 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed comments.


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


Repository: mesos


Description
---

With reservation refinement, we now represent the reservation state in
`Resource.reservations` rather than with `Resource.role` and
`Resource.reservation`. We also keep track of the type of reservation,
and the role the reservation is for in the `Resource.ReservationInfo`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 


Diff: https://reviews.apache.org/r/59861/diff/10/

Changes: https://reviews.apache.org/r/59861/diff/9-10/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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




include/mesos/mesos.proto
Lines 356-357 (patched)


>   // Without this capability, a framework simply is not offered resources
>   // with reservation refinement.

"Without this capability, a framework simply is not offered resources that 
have reservation refinement"



include/mesos/mesos.proto
Lines 835 (patched)


s/with reservation refinements/that have reservation refinement/



include/mesos/mesos.proto
Line 925 (original), 947-950 (patched)


Newline between "sections"



include/mesos/mesos.proto
Lines 969-970 (patched)


`s/Two Resource Formats/Resource Format:/`



include/mesos/mesos.proto
Lines 998 (patched)


Connect.



include/mesos/v1/mesos.proto
Lines 829 (patched)


s/with reservation refinements/that have reservation refinement/



include/mesos/v1/mesos.proto
Line 920 (original), 942-943 (patched)


Copy over the changes from the V0 API.


- Michael Park


On June 15, 2017, 12:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 15, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> `Resource.reservations` rather than with `Resource.role` and
> `Resource.reservation`. We also keep track of the type of reservation,
> and the role the reservation is for in the `Resource.ReservationInfo`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
>   src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/9/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Benjamin Mahler]

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


Ship it!




Modulo comments.

- Benjamin Mahler


On June 15, 2017, 7:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 15, 2017, 7:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> `Resource.reservations` rather than with `Resource.role` and
> `Resource.reservation`. We also keep track of the type of reservation,
> and the role the reservation is for in the `Resource.ReservationInfo`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
>   src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/9/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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




include/mesos/mesos.proto
Lines 980-982 (patched)


Change the formatting of "unreserved" and such to make it not look like a 
JSON field.


- Michael Park


On June 15, 2017, 12:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 15, 2017, 12:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> `Resource.reservations` rather than with `Resource.role` and
> `Resource.reservation`. We also keep track of the type of reservation,
> and the role the reservation is for in the `Resource.ReservationInfo`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
>   src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/9/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Benjamin Mahler]

> On June 15, 2017, 6:59 p.m., Neil Conway wrote:
> > src/slave/constants.cpp
> > Line 40 (original), 40 (patched)
> > 
> >
> > This should add `RESERVATION_REFINEMENT`.

We try to add the agent capability after the agent has the logic corresponding 
to it, before that we test by injecting the capability.


- Benjamin


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


On June 15, 2017, 7:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 15, 2017, 7:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> `Resource.reservations` rather than with `Resource.role` and
> `Resource.reservation`. We also keep track of the type of reservation,
> and the role the reservation is for in the `Resource.ReservationInfo`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
>   src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/9/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Neil Conway]

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




include/mesos/mesos.proto
Lines 347 (patched)


It seems like this capability controls _two_ things:

(1) Whether the framework is offered resources using the new resource 
format.

(2) Whether the framework is offered resources with refined reservations 
and is allowed to refine reservations.

Can we clarify this?



include/mesos/mesos.proto
Lines 833 (patched)


Similarly to the framework capability, would be good to distinguish between 
the data format change from actually taking advantage of refined resources.



include/mesos/mesos.proto
Line 947 (original), 1050 (patched)


"made" rather than "acquired" (the operator doesn't "acquire" the 
reservation).



include/mesos/v1/mesos.proto
Line 939 (original), 962 (patched)


Need to sync with unversioned copy of this file.



src/slave/constants.cpp
Line 40 (original), 40 (patched)


This should add `RESERVATION_REFINEMENT`.


- Neil Conway


On June 15, 2017, 7:37 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 15, 2017, 7:37 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> With reservation refinement, we now represent the reservation state in
> `Resource.reservations` rather than with `Resource.role` and
> `Resource.reservation`. We also keep track of the type of reservation,
> and the role the reservation is for in the `Resource.ReservationInfo`.
> This patch introduces these changes and explains the different formats
> in detail.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
>   include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
>   src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
>   src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
>   src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/9/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for reservation refinement.

[Michael Park]

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

(Updated June 15, 2017, 12:37 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Summary (updated)
-

Added protobuf changes for reservation refinement.


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


Repository: mesos


Description (updated)
---

With reservation refinement, we now represent the reservation state in
`Resource.reservations` rather than with `Resource.role` and
`Resource.reservation`. We also keep track of the type of reservation,
and the role the reservation is for in the `Resource.ReservationInfo`.
This patch introduces these changes and explains the different formats
in detail.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 


Diff: https://reviews.apache.org/r/59861/diff/8/

Changes: https://reviews.apache.org/r/59861/diff/7-8/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 3:26 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Added TODOs for deprecation.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 


Diff: https://reviews.apache.org/r/59861/diff/7/

Changes: https://reviews.apache.org/r/59861/diff/6-7/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 3:21 p.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed bmahler's comments.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 


Diff: https://reviews.apache.org/r/59861/diff/6/

Changes: https://reviews.apache.org/r/59861/diff/5-6/


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 1:15 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
  src/tests/master_tests.cpp 543c4a3afab818578c3124331a092cb1057776af 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 


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

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


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 14, 2017, 1:06 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Addressed some of bmahler's comments.


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


Repository: mesos


Description
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto e9a8b1513150b394f4986a1f34483cfca32b35e3 
  include/mesos/v1/mesos.proto 5e8c15c07a02999e3376d2104c1c642f00511969 
  src/common/protobuf_utils.hpp 7b1a1289c16edde44143432b645ecce68576 
  src/common/protobuf_utils.cpp 46331b379c732edc597b46bb10665c95c9640e96 
  src/slave/constants.cpp d3668e4dca515f42d88d01b0852a543bfdb360f6 
  src/tests/master_tests.cpp 543c4a3afab818578c3124331a092cb1057776af 
  src/tests/slave_tests.cpp 26901f4294ee74e9f8ac57f2b506d369ea540a15 


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

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


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

(Updated June 13, 2017, 1:30 a.m.)


Review request for mesos, Benjamin Mahler and Neil Conway.


Changes
---

Rebased.


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


Repository: mesos


Description (updated)
---

Added protobuf changes for Reservation Refinement.


Diffs (updated)
-

  include/mesos/mesos.proto 31972b09d4bc149d3299264f7404e8471baf84d5 
  include/mesos/v1/mesos.proto cabe0d5d8f47f7078fec9229072cbd5a7c3c4aed 
  src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6 
  src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
  src/slave/constants.cpp 0fbcab8f1cee4183dfd4c25984cd27f8baabda44 
  src/tests/master_tests.cpp 490d7ed4b275ebf5ff6956f7d40dbea3ce3b63e2 
  src/tests/slave_tests.cpp 6aa5d9b673ed1f880a280501afa65797f30665fd 


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

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


Testing
---

Updated tests + `make check`


Thanks,

Michael Park

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Benjamin Mahler]

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




include/mesos/mesos.proto
Lines 347-354 (patched)


Do you want to comment on the semantics with respect to the filtering of 
offers? From our discussion, you were saying that we don't send refined 
reservations (i.e. more than 1 level) to frameworks without the capability.

It doesn't quite seem like this is the intent to create refined 
reservations, they could just want to consume them, or use the new format? It 
would be great to steer them a little more towards this "this enables the new 
format, which has X feature" rather than "provide this if you want to use X 
feature".



include/mesos/mesos.proto
Lines 822-824 (patched)


Not just launch tasks, but handle reservations, volumes, etc, so it's more 
a matter of offer operations?

I see that this was just copy paste from HIERARCHICAL_ROLE, but would be 
good to be more precise here.



include/mesos/mesos.proto
Lines 934 (patched)


I don't think people are going to understand the old vs new format based on 
this NOTE. Could you expand on that here?

E.g. In the old format (pre-XXX capability), role means X and reservation 
is set when X. In the new format (post-XXX capability), role is unused, and 
instead we set 'reservations' when a resource is reserved.

Also, should we have a TODO here for marking 'role' as deprecated?



include/mesos/mesos.proto
Lines 962-965 (patched)


Per [MESOS-4997](https://issues.apache.org/jira/browse/MESOS-4997), could 
you add an 'UNKNOWN = 0' here?

Also, with that high level format comment I mentioned, it would be easier 
for people to understand that we didn't use to have 'Type' when we only had the 
'reservation' field.



include/mesos/mesos.proto
Lines 967-969 (patched)


It's a little tough to figure out the meaning behind this NOTE, if you had 
a high level description of the old and new formats (per my previous comment), 
you could add a reference to that here, so that the reader can understand this 
better.



include/mesos/mesos.proto
Lines 971-973 (patched)


Ditto here w.r.t. the NOTE.



include/mesos/mesos.proto
Lines 998 (patched)


"The stack of reservations"?



include/mesos/mesos.proto
Lines 1006 (patched)


End with a period like the other ones you added?



src/slave/constants.cpp
Lines 41 (patched)


Can we introduce this capability *after* the agent can handle it? We can 
test in the interim by injecting the capability during (re-)registration.



src/tests/master_tests.cpp
Lines 4287-4288 (original), 4287-4288 (patched)


Comment needs updating, ditto below. But, see my above comment.


- Benjamin Mahler


On June 7, 2017, 2:54 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 7, 2017, 2:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5f80170fcd3c05add8b6e9e3107cff062818c1dc 
>   include/mesos/v1/mesos.proto 4b528751006f709f841e44f48c9f5c2dc035b402 
>   src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6 
>   src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
>   src/slave/constants.cpp 0fbcab8f1cee4183dfd4c25984cd27f8baabda44 
>   src/tests/master_tests.cpp 490d7ed4b275ebf5ff6956f7d40dbea3ce3b63e2 
>   src/tests/slave_tests.cpp b5141d7013acdd6e236606ef3d9b1953b14d373a 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/1/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Re: Review Request 59861: Added protobuf changes for Reservation Refinement.

[Mesos Reviewbot Windows]

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



Bad review!

Reviews applied: [59861, 59860, 59859, 58510, 58509, 58508, 57516, 58584, 
57254, 58112, 58110, 57564, 57528, 57527, 57788, 57167, 57166, 56805, 57165, 
57164]

Error:
No reviewers specified. Please find a reviewer by asking on JIRA or the mailing 
list.

- Mesos Reviewbot Windows


On June 7, 2017, 2:54 a.m., Michael Park wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59861/
> ---
> 
> (Updated June 7, 2017, 2:54 a.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and Neil Conway.
> 
> 
> Bugs: MESOS-7575
> https://issues.apache.org/jira/browse/MESOS-7575
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> See summary.
> 
> 
> Diffs
> -
> 
>   include/mesos/mesos.proto 5f80170fcd3c05add8b6e9e3107cff062818c1dc 
>   include/mesos/v1/mesos.proto 4b528751006f709f841e44f48c9f5c2dc035b402 
>   src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6 
>   src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
>   src/slave/constants.cpp 0fbcab8f1cee4183dfd4c25984cd27f8baabda44 
>   src/tests/master_tests.cpp 490d7ed4b275ebf5ff6956f7d40dbea3ce3b63e2 
>   src/tests/slave_tests.cpp b5141d7013acdd6e236606ef3d9b1953b14d373a 
> 
> 
> Diff: https://reviews.apache.org/r/59861/diff/1/
> 
> 
> Testing
> ---
> 
> Updated tests + `make check`
> 
> 
> Thanks,
> 
> Michael Park
> 
>

Review Request 59861: Added protobuf changes for Reservation Refinement.

[Michael Park]

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

Review request for mesos, Benjamin Mahler and Neil Conway.


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


Repository: mesos


Description
---

See summary.


Diffs
-

  include/mesos/mesos.proto 5f80170fcd3c05add8b6e9e3107cff062818c1dc 
  include/mesos/v1/mesos.proto 4b528751006f709f841e44f48c9f5c2dc035b402 
  src/common/protobuf_utils.hpp be2325f05b81b847fa592eff65175cbc99764fd6 
  src/common/protobuf_utils.cpp 3fcaf786b29a00f003c10b0f1614a2c7eddc725d 
  src/slave/constants.cpp 0fbcab8f1cee4183dfd4c25984cd27f8baabda44 
  src/tests/master_tests.cpp 490d7ed4b275ebf5ff6956f7d40dbea3ce3b63e2 
  src/tests/slave_tests.cpp b5141d7013acdd6e236606ef3d9b1953b14d373a 


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


Testing
---

Updated tests + `make check`


Thanks,

Michael Park