> On May 14, 2014, 11:13 a.m., Ben Mahler wrote: > > src/master/master.hpp, lines 519-522 > > <https://reviews.apache.org/r/21166/diff/5/?file=580785#file580785line519> > > > > (1) To keep this simple for now, I would suggest we only add metrics > > for the "grandfathered" scalar resources: > > > > cpus, mem, disk > > > > This lets us manage the lifetime of the Gauges inside 'Metrics' and > > '~Metrics'. > > > > (2) Why do these need to be allocated on the heap?
(1) I considered that but wanted to make it trivial for more resources to be added. The existing code doesn't assume specific resources (except the webui) so I didn't want to restrict things. I agree it would be much simpler though. Do you think a TODO is necessary, or do you think the chance of more resources being added is so slight that we don't need to worry about it? (2) They absolutely don't: I forgot that they were copyable. > On May 14, 2014, 11:13 a.m., Ben Mahler wrote: > > src/master/master.hpp, lines 568-582 > > <https://reviews.apache.org/r/21166/diff/5/?file=580785#file580785line568> > > > > This is not thread safe! You need to use 'defer' to access the members > > of a Process. I do use defer, so these should be safe. The static is there because I need to bind to the dynamic name, which goes away when we assume resources to be a strict set. - Dominic ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/21166/#review42988 ----------------------------------------------------------- On May 13, 2014, 10:52 a.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/21166/ > ----------------------------------------------------------- > > (Updated May 13, 2014, 10:52 a.m.) > > > Review request for mesos and Ben Mahler. > > > Bugs: MESOS-1132 > https://issues.apache.org/jira/browse/MESOS-1132 > > > Repository: mesos-git > > > Description > ------- > > see summary > > > Diffs > ----- > > src/master/master.hpp 923552e3c7f9942d6be7a0446b44e40f53c11433 > src/master/master.cpp 569995c2eaf758f5171c4e0d082d4e519a143025 > > Diff: https://reviews.apache.org/r/21166/diff/ > > > Testing > ------- > > make check > > ran local instance and manually verified resource statistics show up and > match stats.json. > > > Thanks, > > Dominic Hamon > >
