> On Feb. 4, 2013, 9:31 p.m., Benjamin Hindman wrote: > > third_party/libprocess/src/statistics.cpp, lines 60-61 > > <https://reviews.apache.org/r/9090/diff/2/?file=252492#file252492line60> > > > > This is a departure from our standard style ... what caused you to add > > the CHECKs?
I don't need these CHECKs anymore, as originally without the leading '/' these were not routing! (Returning false). As a note, we may want to make this a Try or have all callers use CHECK. As long as callers are not ignoring the return value. > On Feb. 4, 2013, 9:31 p.m., Benjamin Hindman wrote: > > third_party/libprocess/include/process/statistics.hpp, line 14 > > <https://reviews.apache.org/r/9090/diff/2/?file=252490#file252490line14> > > > > I'd prefer to namespace statistics. I had imagined just an instance > > called 'statistics' within the process namespace. Two wins I see here. > > First, people see usages like 'process::statistics.set(...)' and they see > > it's clearly provided by libprocess. Likewise, they might do a 'using > > process::statistics' to just bring in the 'statistics' object from the > > process namespace. I recognize the desire for a dynamically allocated > > instance of Statistics in order to avoid cleanup issues (are there any?). > > Hence, a macro provides a nice means to handle that level of indirection, > > but maybe it's not so bad if people have to do 'statistics->set()'. On the > > flip side, one could imagine adding a statistics namespace that mirrors the > > Statistics interface yielding usage like 'process::statistics::set()'. I > > probably like this best, but recognize the unfortunate need to replicate > > the interface (and keep it in sync, etc). As discussed, added a process::statistics pointer to a heap allocated Statistics object. - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9090/#review16066 ----------------------------------------------------------- On Jan. 28, 2013, 10:49 p.m., Ben Mahler wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9090/ > ----------------------------------------------------------- > > (Updated Jan. 28, 2013, 10:49 p.m.) > > > Review request for mesos, Benjamin Hindman and Vinod Kone. > > > Description > ------- > > This adds a global Statistics object for libprocess, which enables > centralized stats across Processes. > > I'd love to eventually plumb stats into libprocess and export to the webui! > (MESOS-320) > > > This addresses bug MESOS-324. > https://issues.apache.org/jira/browse/MESOS-324 > > > Diffs > ----- > > third_party/libprocess/include/process/statistics.hpp > 9e3041a6e2a8ef022eacacad00bc4d974a8e33c9 > third_party/libprocess/src/process.cpp > 72e437862ee0b35126c16d32bec79ef76a4e2b23 > third_party/libprocess/src/statistics.cpp > 2fe8af83c6c63a0fa8cb2e9636f9289f0e3d7f2f > > Diff: https://reviews.apache.org/r/9090/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Ben Mahler > >
