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/06/14 22:18:18 UTC

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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


Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................

IMPALA-7174: fix test_cancellation for RELEASE builds

The test was DOA when run against a release build because the debug
actions that it depends on were disabled. The fix is to enable the
debug actions for release builds, similar to other debug actions.

I assume the original motivation of the NDEBUG checks was to avoid
adding overhead to release builds. The cost is minimised by quickly
checking whether the string is empty before proceeding with any
further work.

Testing:
Looped the test on a release build.

Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
---
M be/src/scheduling/admission-controller.cc
M be/src/service/client-request-state.cc
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
4 files changed, 12 insertions(+), 17 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 2: Code-Review+2

Sounds fine. And I'll be sure not to revert it with https://gerrit.cloudera.org/#/c/10690/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:45:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/10722 )

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................

IMPALA-7174: fix test_cancellation for RELEASE builds

The test was DOA when run against a release build because the debug
actions that it depends on were disabled. The fix is to enable the
debug actions for release builds, similar to other debug actions.

I assume the original motivation of the NDEBUG checks was to avoid
adding overhead to release builds. The cost is minimised by quickly
checking whether the string is empty before proceeding with any
further work.

Also remove wonky exception handling - the test was swallowing
exceptions but we don't expect that code to throw exceptions.

Testing:
Looped the test on a release build.

Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
---
M be/src/scheduling/admission-controller.cc
M be/src/service/client-request-state.cc
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M tests/custom_cluster/test_admission_controller.py
5 files changed, 14 insertions(+), 21 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 2:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 22:20:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 3: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:55:24 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 2: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:41:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2680/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:54:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2681/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:55:25 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jun 2018 03:21:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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/10722 )

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................

IMPALA-7174: fix test_cancellation for RELEASE builds

The test was DOA when run against a release build because the debug
actions that it depends on were disabled. The fix is to enable the
debug actions for release builds, similar to other debug actions.

I assume the original motivation of the NDEBUG checks was to avoid
adding overhead to release builds. The cost is minimised by quickly
checking whether the string is empty before proceeding with any
further work.

Also remove wonky exception handling - the test was swallowing
exceptions but we don't expect that code to throw exceptions.

Testing:
Looped the test on a release build.

Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Reviewed-on: http://gerrit.cloudera.org:8080/10722
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/scheduling/admission-controller.cc
M be/src/service/client-request-state.cc
M be/src/util/debug-util.cc
M be/src/util/debug-util.h
M tests/custom_cluster/test_admission_controller.py
5 files changed, 14 insertions(+), 21 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 4:

I thought we weren't maintaining the branch. Sent an email to dev@ to clarify


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 19:19:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 3:

Hit HBase flake in data log


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jun 2018 23:57:33 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-7174: fix test cancellation for RELEASE builds

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

Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................


Patch Set 4:

Cherrypick to 2.x? (Not a clean pick)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 10722
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tianyi Wang <tw...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Jun 2018 19:12:32 +0000
Gerrit-HasComments: No