> On Oct. 31, 2013, 10:18 p.m., Ian Downes wrote: > > src/slave/cgroups_isolator.cpp, line 782 > > <https://reviews.apache.org/r/15015/diff/1/?file=372971#file372971line782> > > > > Why don't we correct the naming while we're at it: > > > > total_memory <- memory.usage_in_bytes, this is the primary memory > > statistic > > > > # and then the breakdown: > > total_rss <- total_rss from memory.stat > > total_cached <- total_cache from memory.stat > > total_mapped_file <- total_mapped_file from memory.stat > > > > We probably don't need to include the total_ prefix? > > > > An executor's memory limit then matches with total_memory and, only if > > necessary, one can look at the breakdown.
I don't follow. We absolutely need the total_ prefix when reading the fields from the kernel. And it is the kernel's memory.stat file where the naming is deceptive I can't fix that with a mesos patch. I did do what I can do with a mesos patch which is name the fields in ResourceStatics and in the http interface in a non-deceptive way. We absolutely do not even want to consider passing the kernel names straight through as those names are wrong. Especially the borked total_rss field name as that is totally wrong. The current mapping implemented by this patch is: mem_rss_bytes <- memory.usage_in_bytes mem_anon_bytes <- memory.stat:total_rss mem_file_bytes <- memory.stat:total_cache mem_mapped_file_bytes <- memory.stat:total_mapped_file I would include mem_mapped_anon_bytes but by definition that is exactly the same as mem_anon_bytes. I can see renaming mem_rss_bytes something like mem_usage_bytes, however that is going to break existing users of mesos, and breaking existing users is a bad idea. And resident set size and usage synonyms so I don't see any pressing need for a rename there. - Eric ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15015/#review27967 ----------------------------------------------------------- On Oct. 29, 2013, 6:53 a.m., Eric Biederman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15015/ > ----------------------------------------------------------- > > (Updated Oct. 29, 2013, 6:53 a.m.) > > > Review request for mesos, Ben Mahler, Ian Downes, and Vinod Kone. > > > Bugs: Mesos-758 > https://issues.apache.org/jira/browse/Mesos-758 > > > Repository: mesos-git > > > Description > ------- > > This reports a basic break down of memory usage so jobs whose page > cache expands to their configured hard limit will have information > that allows them to size their jobs. > > This does not include all fields from memory.stat and not even all > total_ fields from memory.stat but rather the fields I have reviewed > and seen a need for, and have a reasonable expectation that the fields > will report useful information and work as designed. > > Quite a few of the fields appear only useful for kernel level debugging. > > In particular the fields: > memory.stat:total_cache which reports the amount of file backed > memory in bytes > memory.stat:total_rss which reports the amount of anonymous memory > in bytes > memory.stat:total_mapped_file which reports the amount of mapped file > memory in bytes > > Reading memory.stat is not quite atomic and neither is computing the > values. As even in the simplest cases the values are sums of per cpu > counters of the various statistics. However for the most part the numbers > are a reasonable > snapshot of the state of the cgroup at a particular instant. > > > Diffs > ----- > > include/mesos/mesos.proto fe1d82b > src/slave/cgroups_isolator.cpp fc36f1f > src/slave/monitor.cpp 9cb6256 > src/tests/monitor_tests.cpp 3d3f8af > > Diff: https://reviews.apache.org/r/15015/diff/ > > > Testing > ------- > > make and make check pass > The code is a trivial extension of what already exists. > > > Thanks, > > Eric Biederman > >
