Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/10394 )
Change subject: IMPALA-110 (part 2): Refactor PartitionedAggregationNode ...................................................................... Patch Set 4: (2 comments) Had a couple of high-level questions first http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/common/global-flags.cc@123 PS4, Line 123: DEFINE_bool(use_legacy_aggregation, false, "(Advanced) If true, fall back to using the " What's the motivation for keeping the old implementation? Are we just concerned about having a fallback in case of bugs, or are there specific things to address before we feel comfortable removing it? I'm open to doing this, mainly want to have a clear plan for eventual removal so we avoid carrying it around for a long time like the old Aggs and Joins. http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/aggregator.h File be/src/exec/aggregator.h: http://gerrit.cloudera.org:8080/#/c/10394/4/be/src/exec/aggregator.h@95 PS4, Line 95: /// Account for peak memory used by this aggregator. Is the idea here to account for memory used by each aggregator separately? That's probably a useful property but wanted to make sure I understood the motivation (the alternative would be to have more of the MemPools, etc in the ExecNode). -- To view, visit http://gerrit.cloudera.org:8080/10394 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9e7bb583f54aa4add3738bde7f57cf3511ac567e Gerrit-Change-Number: 10394 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Alex Behm <alex.b...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Thomas Marshall <thomasmarsh...@cmu.edu> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Vuk Ercegovac <vercego...@cloudera.com> Gerrit-Comment-Date: Fri, 22 Jun 2018 21:40:39 +0000 Gerrit-HasComments: Yes
