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

Reply via email to