Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10884 )
Change subject: IMPALA-7239: Disable smaps parsing by default ...................................................................... Patch Set 1: (3 comments) I don't think it's a breaking change - it seems reasonable to expect that metrics collection tools should be able to handle missing data. It's sucky to add something useful then remove it but seems like the lesser evil at the moment. http://gerrit.cloudera.org:8080/#/c/10884/1/be/src/util/memory-metrics.cc File be/src/util/memory-metrics.cc: http://gerrit.cloudera.org:8080/#/c/10884/1/be/src/util/memory-metrics.cc@35 PS1, Line 35: enable_all_memory_metrics > instead of "all" perhaps "smaps" or "extended" or something? Done http://gerrit.cloudera.org:8080/#/c/10884/1/be/src/util/memory-metrics.cc@124 PS1, Line 124: AggregateMemoryMetrics::NUM_MAPS = > if we still care about NUM_MAPS we could parse /proc/self/maps instead of s I did some experiments on a CentOS7 system where we saw some perf issues and the sys time to parse /maps is actually about the same as /smaps. I think the same O(N^2) bug is present because it attributes stacks to threads. http://gerrit.cloudera.org:8080/#/c/10884/1/be/src/util/memory-metrics.cc@128 PS1, Line 128: AggregateMemoryMetrics::RSS = aggregate_metrics->AddGauge("memory.rss", 0U); > RSS is available in /proc/self/status if we care We already have utilities to parse /status so I switched to using that. I think a lot of monitoring tools already have good ways to measure RSS and VM size so those are probably of lesser importance but it's nice having this available still. -- To view, visit http://gerrit.cloudera.org:8080/10884 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I235eddde8fe925866e0581b235752354a3f36d5b Gerrit-Change-Number: 10884 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Comment-Date: Fri, 06 Jul 2018 20:40:08 +0000 Gerrit-HasComments: Yes
