Zoltan Borok-Nagy 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) Did an initial pass, lgtm, only had some minor comments 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 max peak value of the backends. But I guess it's already too late to change this semantic. I see that we cannot easily calculate what was the peak memory consumption for a query since the memory consumption is fluctuating over time on the different backends. But still, if we sum all the peak values, we can get some upper bound which tells that the query never used more memory of the cluster than this upper bound. However, I don't really know how helpful it is. Anyway, since we have these metrics per backend, we can calculate these values anytime. 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: 1'000'000'000L 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. -- 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 13:24:52 +0000 Gerrit-HasComments: Yes
