Dan Hecht has posted comments on this change. Change subject: IMPALA-4674: Part 2: port backend exec to BufferPool ......................................................................
Patch Set 30: Code-Review+2 (4 comments) +2 assuming we do the addition flag in a separate change. Good luck with the checkin. http://gerrit.cloudera.org:8080/#/c/5801/30/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: PS30, Line 1246: nitialise typo http://gerrit.cloudera.org:8080/#/c/5801/30/be/src/runtime/exec-env.cc File be/src/runtime/exec-env.cc: Line 366: << PrettyPrinter::Print(bytes_limit, TUnit::BYTES); maybe log bufffer_pool_capacity as well? or do we do that already somewhere else? http://gerrit.cloudera.org:8080/#/c/5801/26/be/src/runtime/query-state.cc File be/src/runtime/query-state.cc: PS26, Line 149: bytes_limit = query_options().mem_limit; > hmm i'm not sure, maybe. What about the case where there is no query mem_l How about we file a jira for this and consider it for a follow on change? or do you want to settle it in this commit? http://gerrit.cloudera.org:8080/#/c/5801/29/tests/query_test/test_mem_usage_scaling.py File tests/query_test/test_mem_usage_scaling.py: Line 126: 'Q21' : 187, 'Q22' : 125} > Part of it is that the values were either stale or obtained in a different Is that because we have to assume all fragment executions overlap, or pessimism within a single fragment, or both? anyway, I added a comment to the IMPALA-3200 test doc, we can discuss there. -- To view, visit http://gerrit.cloudera.org:8080/5801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7fc7fe1c04e9dfb1a0c749fb56a5e0f2bf9c6c3e Gerrit-PatchSet: 30 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes
