> 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? > > Ben Mahler wrote: > 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!
> 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 agree. To be specific, my worry is that it'll be confusing for `usedResources` to be not exposed as "used_resources". If you think that wouldn't be a big deal, I'm happy to change the naming scheme. Perhaps we can simply have a `NOTE` to help avoid the confusion. - Michael ----------------------------------------------------------- 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 > >
