You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Michael Park <mc...@gmail.com> on 2014/12/17 20:37:31 UTC
Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/
-----------------------------------------------------------
Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
Repository: mesos-git
Description
-------
Add dynamic reservation information to LaunchTasksMessage.
Diffs
-----
src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
Diff: https://reviews.apache.org/r/29173/diff/
Testing
-------
Thanks,
Michael Park
Re: Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
Posted by Jie Yu <yu...@gmail.com>.
> On Dec. 19, 2014, 10:13 p.m., Ben Mahler wrote:
> > src/messages/messages.proto, lines 178-179
> > <https://reviews.apache.org/r/29173/diff/1/?file=794998#file794998line178>
> >
> > Jie and I took a look at this API. Here are some thoughts:
> >
> > From your other patches, it looks like 'release' and 'acquire' both specify the "before" state of the Resources. This works for now but will stop working the moment we have multi-role frameworks, for example. At that point, we would need the "after" state for acquisition, in order to correctly identify the desired dynamic role.
> >
> > It also is a bit implicit in that it is forced to conflate the release of, say, a dynamically reserved persistent disk. In this case, you must release both the persistent disk and the dynamic reservation, because we only take the "before" state of the resources.
> >
> > So, here's a proposal:
> >
> >
> > Introduce an 'Operation' (feel free to propose a better name!) message nested inside Resource that explicitly has the following:
> >
> > ```
> > message Resource {
> >
> > message Operation {
> > enum Type {
> > DYNAMIC_RESERVATION_ACQUISITION = 1;
> > DYNAMIC_RESERVATION_RELEASE = 2;
> > PERSISTENT_DISK_ACQUISITION = 3;
> > PERSISTENT_DISK_RELEASE = 4;
> > }
> >
> > required Type type = 1;
> >
> > repeated Resource before = 2;
> > repeated Resource after = 3;
> > }
> >
> > }
> >
> > message LaunchTaskMessage {
> > ...
> >
> > repeated Resource::Operation operations;
> > }
> > ```
> >
> > The nice part about this is that we can make the kinds of operations very explicit in the API, we can make the validation code very simple, and we remove the ambiguous "inference" about the type of operation. Thoughts?
To add to Ben's point, another advantange for using a single ordered 'operations' in LaunchTaskMessage is because we make the resources transformation order very explicit. Otherwise, if you have both 'acquire' and 'release', you implicitly have an ordering constraint that 'release' will be applied first before 'acquire' (which is not very obvious and explicit to the framework).
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/#review65679
-----------------------------------------------------------
On Dec. 17, 2014, 7:38 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29173/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 7:38 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-2138
> https://issues.apache.org/jira/browse/MESOS-2138
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add dynamic reservation information to LaunchTasksMessage.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
>
> Diff: https://reviews.apache.org/r/29173/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/#review65679
-----------------------------------------------------------
Hey Michael, jie and I went over this and since Jie needs this as well, he will be introducing this stuff based on the proposal below:
src/messages/messages.proto
<https://reviews.apache.org/r/29173/#comment108891>
Jie and I took a look at this API. Here are some thoughts:
From your other patches, it looks like 'release' and 'acquire' both specify the "before" state of the Resources. This works for now but will stop working the moment we have multi-role frameworks, for example. At that point, we would need the "after" state for acquisition, in order to correctly identify the desired dynamic role.
It also is a bit implicit in that it is forced to conflate the release of, say, a dynamically reserved persistent disk. In this case, you must release both the persistent disk and the dynamic reservation, because we only take the "before" state of the resources.
So, here's a proposal:
Introduce an 'Operation' (feel free to propose a better name!) message nested inside Resource that explicitly has the following:
```
message Resource {
message Operation {
enum Type {
DYNAMIC_RESERVATION_ACQUISITION = 1;
DYNAMIC_RESERVATION_RELEASE = 2;
PERSISTENT_DISK_ACQUISITION = 3;
PERSISTENT_DISK_RELEASE = 4;
}
required Type type = 1;
repeated Resource before = 2;
repeated Resource after = 3;
}
}
message LaunchTaskMessage {
...
repeated Resource::Operation operations;
}
```
The nice part about this is that we can make the kinds of operations very explicit in the API, we can make the validation code very simple, and we remove the ambiguous "inference" about the type of operation. Thoughts?
- Ben Mahler
On Dec. 17, 2014, 7:38 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29173/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 7:38 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-2138
> https://issues.apache.org/jira/browse/MESOS-2138
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add dynamic reservation information to LaunchTasksMessage.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
>
> Diff: https://reviews.apache.org/r/29173/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/#review65371
-----------------------------------------------------------
Patch looks great!
Reviews applied: [29173]
All tests passed.
- Mesos ReviewBot
On Dec. 17, 2014, 7:38 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29173/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 7:38 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-2138
> https://issues.apache.org/jira/browse/MESOS-2138
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add dynamic reservation information to LaunchTasksMessage.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
>
> Diff: https://reviews.apache.org/r/29173/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/#review65361
-----------------------------------------------------------
src/messages/messages.proto
<https://reviews.apache.org/r/29173/#comment108469>
Perhaps for dynamic reservations I should move away from *release/acquire* and use *unreserve/reserve* instead?
- Michael Park
On Dec. 17, 2014, 7:38 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/29173/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 7:38 p.m.)
>
>
> Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
>
>
> Bugs: MESOS-2138
> https://issues.apache.org/jira/browse/MESOS-2138
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Add dynamic reservation information to LaunchTasksMessage.
>
>
> Diffs
> -----
>
> src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
>
> Diff: https://reviews.apache.org/r/29173/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 29173: [WIP] Add dynamic reservation information to
LaunchTasksMessage.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29173/
-----------------------------------------------------------
(Updated Dec. 17, 2014, 7:38 p.m.)
Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and Vinod Kone.
Bugs: MESOS-2138
https://issues.apache.org/jira/browse/MESOS-2138
Repository: mesos-git
Description
-------
Add dynamic reservation information to LaunchTasksMessage.
Diffs
-----
src/messages/messages.proto 6b261f7815fcc8ff80fc49d4804ecb72614a14f3
Diff: https://reviews.apache.org/r/29173/diff/
Testing
-------
Thanks,
Michael Park