> On April 15, 2015, 7:20 p.m., Ben Mahler wrote: > > src/master/master.hpp, lines 1138-1143 > > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1138> > > > > Thanks for this NOTE! We might want to take this opportunity to > > articulate why we can safely keep the totals. Or, should we strip > > non-scalars and store a 'totalUsedScalarResources'? > > Michael Park wrote: > We can't do the former since we can't safely keep the totals. We might > want to do something like the latter but I think we may break some people > that way, and therefore should be done as a subsequent task. I've created > [MESOS-2623](https://issues.apache.org/jira/browse/MESOS-2623) to track this.
> We can't do the former since we can't safely keep the totals. Ah ok great, can we articulate why for posterity? It's not immediately obvious to me. As in, is there a bug due to this? Or is it just that the non-scalar information is not correct, and there's not yet a bug due to this since we only expose scalars? Something else? > On April 15, 2015, 7:20 p.m., Ben Mahler wrote: > > src/master/master.hpp, lines 1145-1147 > > <https://reviews.apache.org/r/31665/diff/5/?file=927382#file927382line1145> > > > > How about s/usedResources/totalUsedResources/ and > > s/usedResourcesBySlaveId/usedResources/ ? > > > > This might also make the "total" referred to in your note a bit > > clearer. Ditto for offered. > > Michael Park wrote: > Yeah, I considered `totalUsedResources` and `usedResources` and I liked > those. I went with `usedResources` and `usedResourcesBySlaveId` because I > thought exposing `totalUsedResources` via `used_resources`, then having to > expose `usedResources` via some other key like `used_resources_by_slave_id` > would create confusion later on. This way `usedResources` maps to > `used_resources` and `usedResourcesBySlaveId` can map to > `used_resources_by_slave_id`. Unless we decide that we're never going to > expose `usedResourcesBySlaveId`, in which case we'll have > `totalUsedResources` mapped to `used_resources` and that'll be it. > > What do you think? That makes sense, although, since we both had the same initial intuition, others may as well. I think it's ok not to allow the JSON format to affect our variable naming, since we want to make sure that the code is as readable / understandable as possible. I like the clarified total and used naming scheme you considered initially because it allows us to make it clear that "used resources" have to be namespaced by slaves to be correctly represented, whereas the total only makes sense for scalars (we can have a note about that). Someone is more likely to ponder this, especially if it's called `'totalUsedScalarResources'`. Feel free to punt on stripping non-scalars if you want to tackle it later! - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31665/#review80252 ----------------------------------------------------------- On April 15, 2015, 10:45 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31665/ > ----------------------------------------------------------- > > (Updated April 15, 2015, 10:45 p.m.) > > > Review request for mesos, Alexander Rukletsov and Ben Mahler. > > > Bugs: MESOS-2373 > https://issues.apache.org/jira/browse/MESOS-2373 > > > Repository: mesos > > > Description > ------- > > `master::Framework` holds 2 member variables of type `Resources`: > `usedResources` and `offeredResources`. Both of these are aggregates of > resources from multiple slaves. We add the `hashmap<SlaveID, Resources>` > versions since we can lose non-scalar resources by summing them up across > multiple slaves. For further details refer to > [MESOS-2373](https://issues.apache.org/jira/browse/MESOS-2373). > > In [r31666](https://reviews.apache.org/r/31666/), we report > `usedResourcesBySlaveId` rather than `usedResources` when adding the > framework to the allocator. > > We don't actually use `offeredResourcesBySlaveId` currently, but it should be > kept for symmetry. There will be a ticket to expose `usedResourcesBySlaveId` > as well as `offeredResourcesBySlaveId` via the HTTP endpoint. > > > Diffs > ----- > > src/master/master.hpp 6141917644b84edfed9836fa0a005d55a36880e3 > > Diff: https://reviews.apache.org/r/31665/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Michael Park > >
