> 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 > >
