You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tim Armstrong (Code Review)" <ge...@cloudera.org> on 2018/02/23 17:36:03 UTC

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Tim Armstrong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9425


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

IMPALA-6482: add EXEC_TIME_LIMIT_S option

This is similar to the QUERY_TIMEOUT_S option and shares most of the
implementation. The difference is that the timeout doesn't reset at
any point.

The time limit is measured from the start of query execution,
after the query is admitted, so planning, scheduling and
time spent in admission control is not counted towards the time limit.

Also fix validation of the related QUERY_TIMEOUT_S option, which
previously could ignore invalid input.

Testing:
Added tests for various permutations:
* With and without query_timeout_s set
* With and without result fetching keeping the query active

Conflicts:
	be/src/service/query-options.cc
	be/src/service/query-options.h

Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_query_expiration.py
10 files changed, 303 insertions(+), 116 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/9425/1
-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2004/


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 20:34:18 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 24 Feb 2018 01:32:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 17:48:30 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/9425

to look at the new patch set (#2).

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

IMPALA-6482: add EXEC_TIME_LIMIT_S option

This is similar to the QUERY_TIMEOUT_S option and shares most of the
implementation. The difference is that the timeout doesn't reset at
any point.

The time limit is measured from the start of query execution,
after the query is admitted, so planning, scheduling and
time spent in admission control is not counted towards the time limit.

Also fix validation of the related QUERY_TIMEOUT_S option, which
previously could ignore invalid input.

Testing:
Added tests for various permutations:
* With and without query_timeout_s set
* With and without result fetching keeping the query active

Conflicts:
	be/src/service/query-options.cc
	be/src/service/query-options.h

Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_query_expiration.py
10 files changed, 304 insertions(+), 111 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/9425/2
-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 1:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2002/


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Fri, 23 Feb 2018 17:36:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9425 )

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

IMPALA-6482: add EXEC_TIME_LIMIT_S option

This is similar to the QUERY_TIMEOUT_S option and shares most of the
implementation. The difference is that the timeout doesn't reset at
any point.

The time limit is measured from the start of query execution,
after the query is admitted, so planning, scheduling and
time spent in admission control is not counted towards the time limit.

Also fix validation of the related QUERY_TIMEOUT_S option, which
previously could ignore invalid input.

Testing:
Added tests for various permutations:
* With and without query_timeout_s set
* With and without result fetching keeping the query active

Conflicts:
	be/src/service/query-options.cc
	be/src/service/query-options.h

Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Reviewed-on: http://gerrit.cloudera.org:8080/9425
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
Tested-by: Impala Public Jenkins
---
M be/src/service/client-request-state.cc
M be/src/service/client-request-state.h
M be/src/service/impala-server.cc
M be/src/service/impala-server.h
M be/src/service/query-options-test.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M tests/custom_cluster/test_query_expiration.py
10 files changed, 304 insertions(+), 111 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 1: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/2002/


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 21:24:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 1:

Had to fix a couple of conflicts resulting from changed surrounding lines in query-options.*


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 17:37:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR](2.x) IMPALA-6482: add EXEC TIME LIMIT S option

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/9425 )

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


Patch Set 2: Code-Review+2

Fix merge error


-- 
To view, visit http://gerrit.cloudera.org:8080/9425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: Id81772ee223ffb64746e241027a5a734a811e1b8
Gerrit-Change-Number: 9425
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Feb 2018 20:34:09 +0000
Gerrit-HasComments: No