Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9227 )
Change subject: IMPALA-6482: add QUERY_TIME_LIMIT_S option ...................................................................... Patch Set 5: (2 comments) I also considered calling this something like QUERY_EXEC_TIME_LIMIT_S to hint that it doesn't include planning, admission, etc, but it seemed a little verbose. http://gerrit.cloudera.org:8080/#/c/9227/5/be/src/service/impala-server.h File be/src/service/impala-server.h: http://gerrit.cloudera.org:8080/#/c/9227/5/be/src/service/impala-server.h@1035 PS5, Line 1035: // The query is cancelled if the query has not been inactive this long. The event > "has not been inactive" is confusing. Is the "not" there correct? oops, had an unintended double negative http://gerrit.cloudera.org:8080/#/c/9227/5/common/thrift/ImpalaInternalService.thrift File common/thrift/ImpalaInternalService.thrift: http://gerrit.cloudera.org:8080/#/c/9227/5/common/thrift/ImpalaInternalService.thrift@271 PS5, Line 271: // Time, in s, before a query will be timed out after it starts executing. > Mention explicitly that this doesn't include waiting for admission? Done, here and in the other .thrift -- To view, visit http://gerrit.cloudera.org:8080/9227 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8 Gerrit-Change-Number: 9227 Gerrit-PatchSet: 5 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Philip Zeyliger <phi...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Tue, 20 Feb 2018 00:34:46 +0000 Gerrit-HasComments: Yes