> On March 15, 2018, 7:10 p.m., Chun-Hung Hsiao wrote: > > include/mesos/mesos.proto > > Lines 1975 (patched) > > <https://reviews.apache.org/r/66049/diff/1/?file=1974927#file1974927line1975> > > > > I'm thinking that, instead of asking the framework to craft the freed > > disk, we could leave it to the agent/RP so they have the freedom to > > transform the freed disk to an appropriate type of disk resource (although > > for now it will be the same as the original volume except in size and > > persistence). So how about having a `Scalar target` instead, and we can > > implement it through `Resources::shrink()`? > > Zhitao Li wrote: > I suggest we do not look at how `Resources` class implements various > helpers when designing public API, but rather think about how to make the API > easy, straightforward and consistent for the users (here it would be > framework authors). > > Initially, I chose this format because it is more symetrical to > `RESERVE`/`UNRESERVE` and could be chained together in one operation. > However, after the decision to make new API non-chainable (and > non-speculative), that argument seems weaker now. > > If we go with `target` format, I'd rather make the target as a full > `Resource` object and the API will look like: > > ``` > message SHRINK_VOLUME { > required Resource original = 1; // original volume resource > required Resource target = 2; // target volume resource > } > ``` > > Framework author can just make a copy of `original` and change the scalar > quantity. > > Eventually, we might be able to mark `original` optional or drop it in > the API. > > What do you think?
Yeah you're right about not making API decision based on utility functions. Dropping `original` doesn't sound a good idea to me for the following reasons: 1. Consistency-wise, it would be different from other non-speculative calls such as `CREATE_VOLUME` or `DESTROY_VOLUME` (NOTE: I'm not talking about the `CREATE_VOLUMES` or `DESTROY_VOLUMES` operator calls), where we specify the source resource and let the operation performer to craft the converted.results. 2. Implementation-wise, the master and agent still need to find the original resource to construct and apply the resource conversion. If that doesn't come from the request, we need to do an extra search to find the source, which I don't feel necessary. For `target` scalar vs resource, we could think about what are the restrictions we'd like to enforce. If we want to make sure that the resulting shrunk resource must look exactly the same and have no other metadata change, then I'm fine with making it a `Reaource`. If we want to have minimal restrictions and give the operation performer some freedom to modify the resoure (for example, make `taeget` a reference size or upper limit and thus the agent can shrink the volume as small as possible), then I feel a scalar looks better. FYI, in `CREATE_VOLUME`, we specify a `source` and a target type (`MOUNT` or `PATH`) only instead of the resulting resource, because the reaource provider will fill in extra information such as `id` and `metadata`. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66049/#review199283 ----------------------------------------------------------- On March 13, 2018, 10:48 p.m., Zhitao Li wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66049/ > ----------------------------------------------------------- > > (Updated March 13, 2018, 10:48 p.m.) > > > Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman. > > > Bugs: MESOS-4965 > https://issues.apache.org/jira/browse/MESOS-4965 > > > Repository: mesos > > > Description > ------- > > Added offer operation to grow and shrink persistent volumes. > > > Diffs > ----- > > include/mesos/mesos.proto e6ba3746456c9241ceaefac39200f68562dd5cb9 > include/mesos/v1/mesos.proto 30d4d35e865db2af1ba85b12e2b5b0e499ef8de8 > > > Diff: https://reviews.apache.org/r/66049/diff/1/ > > > Testing > ------- > > > Thanks, > > Zhitao Li > >
