Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10989 )
Change subject: IMPALA-7256: Aggregator mem usage isn't reflected in summary ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10989/1/be/src/exec/exec-node.cc File be/src/exec/exec-node.cc: http://gerrit.cloudera.org:8080/#/c/10989/1/be/src/exec/exec-node.cc@113 PS1, Line 113: reservation_tracker_.reset(new ReservationTracker); The parallel MemTracker/ReservationTracker hierarchy is pretty annoying here. The reservation reporting won't work properly if ExecNode::reservation_tracker_ and ExecNode::buffer_pool_client_ are both in use because MemTracker assumes it only has one linked ReservationTracker. The design of the trackers feels suboptimal to me but we ended up in this place for some valid reasons. Anyway, I'm thinking it's probably best if we just add reservation_tracker_ to the aggregation nodes where it's actually used so that it's more obvious that this potential wonkiness doesn't happen. -- To view, visit http://gerrit.cloudera.org:8080/10989 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iba6ef207bed47810fc742aec3481db5f313cf97f Gerrit-Change-Number: 10989 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 19 Jul 2018 21:18:54 +0000 Gerrit-HasComments: Yes
