> On April 16, 2015, 7:27 p.m., Ben Mahler wrote: > > src/master/master.hpp, line 1071 > > <https://reviews.apache.org/r/31665/diff/6/?file=931105#file931105line1071> > > > > For example, could we document in NOTE on these total variables that > > subtraction is safe even though non-scalars lose information? > > > > Specifically, is my understanding correct here: > > > > (1) For overlapping set items / ranges across slaves, these will get > > added N times but only represented 1 time in the Resources. > > (2) So, when an initial subtraction occurs (N-1), the resource is no > > longer represented. We don't expose non-scalars for now, so this is safe > > for now. > > (3) When any further subtractions occur (N-(1+M)), the Resources simply > > ignores the subtraction since there's nothing to remove, so this is safe > > for now. > > Michael Park wrote: > I think this was meant as a subsequent comment to the one above, so I'll > answer here. > > I'm not sure if I correctly understand your definition of __safety__ and > what's considered a __bug__. > > (1) For overlapping set items / ranges across slaves, these will get > added N times but only represented 1 time in the Resources. > > Correct. > > (2) So, when an initial subtraction occurs (N-1), the resource is no > longer represented. > > Correct. > > We don't expose non-scalars for now, so this is safe for now. > > We actually do expose non-scalars through the master HTTP endpoint. We > used to only expose `cpus`, `mem`, `disk`, and `ports` but since > [r31082](https://reviews.apache.org/r/31082) we expose all resources. But > even before, `ports` was reported incorrectly. If we mean __safe__ as in we > don't crash, then yes it's safe. But I would probably consider this to be at > least a __bug__. > > (3) When any further subtractions occur (N-(1+M)), the Resources simply > ignores the subtraction since there's nothing to remove, so this is safe for > now. > > Correct. > > After the sequence of patches land, the totals (used and offered) are > merely used to be exposed via `state.json`. The non-scalars are still > incorrect, but I'm not sure that we can assume that nobody's using them. If > we decide to only expose the scalar resources since the non-scalar > information was wrong anyway, I think that should definitely be done in a > separate patch at least to keep the commits clean (as I mentioned, MESOS-2623 > will track that). Furthermore, I think changing what resources get exposed > via `state.json` is considered an API change and therefore is probably > worthwhile mentioning in the `CHANGELOG` for the next release.
I'll add a `NOTE` for why keeping the totals is __safe__ followed by a `TODO` to remove the non-scalars to avoid reporting incorrect totals. - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31665/#review80370 ----------------------------------------------------------- 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 > >
