Dan Hecht has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9227 )

Change subject: IMPALA-6482: add EXEC_TIME_LIMIT_S option
......................................................................


Patch Set 9: Code-Review+2

(9 comments)

http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.h
File be/src/service/impala-server.h:

http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.h@1038
PS9, Line 1038:  a query
query execution


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc
File be/src/service/impala-server.cc:

http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@984
PS9, Line 984:  has timeout of
might be nice to make these more specific:
has an idle timeout of


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@990
PS9, Line 990: has time limit
has an execution time limit of


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@1848
PS9, Line 1848: If the last-observed expiration time for this query is still in 
the future,
not your change, but this seems wrong given that queries_by_timestamp has 
deadlines for various queries right?


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@1864
PS9, Line 1864: time
execution time?


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@1865
PS9, Line 1865: time
execution time?


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@1872
PS9, Line 1872:
DCHECK(kind == IDLE_TIMEOUT)


http://gerrit.cloudera.org:8080/#/c/9227/9/be/src/service/impala-server.cc@1874
PS9, Line 1874: it
last_active_ms?


http://gerrit.cloudera.org:8080/#/c/9227/9/tests/custom_cluster/test_query_expiration.py
File tests/custom_cluster/test_query_expiration.py:

http://gerrit.cloudera.org:8080/#/c/9227/9/tests/custom_cluster/test_query_expiration.py@88
PS9, Line 88:     self.assert_impalad_log_contains('INFO', "Expiring query due 
to client inactivity: "
            :         "[0-9a-f]+:[0-9a-f]+, last activity was at: 
\d\d\d\d-\d\d-\d\d \d\d:\d\d:\d\d")
            :     self.assert_impalad_log_contains('INFO',
            :         "Expiring query [0-9a-f]+:[0-9a-f]+ due to time limit of 
1s")
why do we do that instead of checking the query status message? which we do 
below with __expect_expired, no?



--
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: 9
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@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: Fri, 23 Feb 2018 00:06:32 +0000
Gerrit-HasComments: Yes

Reply via email to