Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11081 )
Change subject: IMPALA-6034: Add scanned bytes limits per query ...................................................................... Patch Set 1: (3 comments) Thanks for the review. Unfortunately I had to rebase in the middle of addressing comments to get Impala to build http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/runtime/coordinator.h File be/src/runtime/coordinator.h: http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/runtime/coordinator.h@173 PS1, Line 173: this is the max : /// peak value from any backend. > To me it's a bit weird that the peak memory consumption for a query is the I updated the name to reflect that it's a per-host value - the shortened name is ambiguous. It is an important value to track because the main memory limit (mem_limit) is also a per-host value. The total aggregate memory is somewhat interesting for understanding the total resource consumption, but still for many purposes <mem_limit> * <number of hosts> is a better measure since with memory-based admission control the whole mem_limit is effectively reserved for the query. http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/service/impala-server.cc File be/src/service/impala-server.cc: http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/service/impala-server.cc@2096 PS1, Line 2096: 1000L * 1000L * 1000L; > nit: you can use digit separators since C++14: Thanks for the pointer http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/service/query-options-test.cc File be/src/service/query-options-test.cc: http://gerrit.cloudera.org:8080/#/c/11081/1/be/src/service/query-options-test.cc@266 PS1, Line 266: int64_t(range.upper_bound) + 1 > int64_t(I64_MAX) + 1 is undefined behavior I think. Done -- To view, visit http://gerrit.cloudera.org:8080/11081 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3e85f80b70b3fce47e637e9322ed0316ee84f6a9 Gerrit-Change-Number: 11081 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 06 Aug 2018 19:36:57 +0000 Gerrit-HasComments: Yes
