[Impala-ASF-CR] Draft: Add JSON output to MemTracker::LogUsage()

[Lars Volker (Code Review)]

Lars Volker has abandoned this change. ( http://gerrit.cloudera.org:8080/8955 )

Change subject: Draft: Add JSON output to MemTracker::LogUsage()
..


Abandoned
--
To view, visit http://gerrit.cloudera.org:8080/8955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: abandon
Gerrit-Change-Id: I0a59c39b0a85c78f1e8db10a93cbb814dc6adfa0
Gerrit-Change-Number: 8955
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Lars Volker 
Gerrit-Reviewer: Michael Ho 

[Impala-ASF-CR] Draft: Add JSON output to MemTracker::LogUsage()

[Csaba Ringhofer (Code Review)]

Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/8955 )

Change subject: Draft: Add JSON output to MemTracker::LogUsage()
..


Patch Set 1:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.h
File be/src/runtime/mem-tracker.h:

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.h@339
PS1, Line 339:   void LogUsage(int max_recursive_depth, std::stringstream* ss, 
const std::string& prefix,
 :   rapidjson::Value* val, rapidjson::Document* doc, int64_t* 
logged_consumption);
 :   /// Convenience wrapper around the previous declaration to 
make it easier to use in
 :   /// DCHECKs.
 :   std::string LogUsage(int max_recursive_depth);
I saw a place where the old interface is used with prefix and 
logged_consumption: 
https://github.com/apache/impala/blob/3302abaa32cf3290cb038fbc1dbd0a821f5cbbc7/be/src/runtime/bufferpool/reservation-tracker-test.cc#L543


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc
File be/src/runtime/mem-tracker.cc:

http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@256
PS1, Line 256: void MemTracker::LogUsage(int max_recursive_depth, 
std::stringstream* ss,
I think that this became a bit too complicated for a log function. My idea for 
an alternative solution is creating a struct with the variables we want to log, 
and split this function to two somewhat less complex ones: one that walks the 
tree and collects these structs (essentially creating a snapshot), and one that 
serializes them into string/json. While this would mean some extra copies, a 
potential performance benefit could be that string formatting would happen when 
no child_trackers_lock_ is locked. If the structs would have a depth field, 
then it would be enough to collect them to a vector (without loosing the tree 
structure).


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@301
PS1, Line 301:   val->AddMember("gauges", gauges, doc->GetAllocator());
if (val != nullptr)


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@309
PS1, Line 309:   string child_trackers_usage;
This variable is no longer used.


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@315
PS1, Line 315: val->AddMember("children", children, doc->GetAllocator());
if (val != nullptr)


http://gerrit.cloudera.org:8080/#/c/8955/1/be/src/runtime/mem-tracker.cc@340
PS1, Line 340:   vector usage_strings;
This variable is no longer used.



--
To view, visit http://gerrit.cloudera.org:8080/8955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0a59c39b0a85c78f1e8db10a93cbb814dc6adfa0
Gerrit-Change-Number: 8955
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Michael Ho 
Gerrit-Comment-Date: Mon, 05 Mar 2018 13:24:02 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] Draft: Add JSON output to MemTracker::LogUsage()

[Lars Volker (Code Review)]

Lars Volker has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/8955


Change subject: Draft: Add JSON output to MemTracker::LogUsage()
..

Draft: Add JSON output to MemTracker::LogUsage()

TODO: Function comments in mem-tracker.h may need updating.

Change-Id: I0a59c39b0a85c78f1e8db10a93cbb814dc6adfa0
---
M be/src/runtime/mem-tracker.cc
M be/src/runtime/mem-tracker.h
M be/src/util/default-path-handlers.cc
M tests/verifiers/mem_usage_verifier.py
4 files changed, 83 insertions(+), 45 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/8955/1
--
To view, visit http://gerrit.cloudera.org:8080/8955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0a59c39b0a85c78f1e8db10a93cbb814dc6adfa0
Gerrit-Change-Number: 8955
Gerrit-PatchSet: 1
Gerrit-Owner: Lars Volker