Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10863 )
Change subject: IMPALA-7240: Fix missing QueryMaintenance call in AddBatchStreaming ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc File be/src/exec/grouping-aggregator.cc: http://gerrit.cloudera.org:8080/#/c/10863/1/be/src/exec/grouping-aggregator.cc@407 PS1, Line 407: SCOPED_TIMER(build_timer_); Do we have a similar bug here? I'm not sure how expr_results_pool_ gets cleared in the aggregator when driven from Open(). Same for the non-grouping aggregator. It would be nice to avoid the need for these two sets of QueryMaintenance() calls to clear expr_results_pool_ at both levels. We could consider using ExecNode::expr_results_pool_, which reduces the granularity of some of the memory tracking but I think should be ok - the "result" allocations are expected to be small unless an expr misbehaves. Otherwise we could have the parent ExecNode() clear the results pools on the aggregators at the same points as it's clearing its own pools to avoid this duplication. -- To view, visit http://gerrit.cloudera.org:8080/10863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I150a46d00acad139d186a150d9706ef47a10ac36 Gerrit-Change-Number: 10863 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 04 Jul 2018 02:37:36 +0000 Gerrit-HasComments: Yes
