You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Quanlong Huang (Code Review)" <ge...@cloudera.org> on 2019/02/16 00:25:42 UTC
[Impala-ASF-CR](2.x) IMPALA-7174: fix test cancellation for RELEASE builds
Hello Impala Public Jenkins,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/12504
to review the following change.
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(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/12504/1
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR](2.x) 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/12504 )
Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 16:48:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) 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/12504 )
Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/2145/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 01:09:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7174: fix test cancellation for RELEASE builds
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12504 )
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>
Reviewed-on: http://gerrit.cloudera.org:8080/12504
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Reviewed-by: Tim Armstrong <ta...@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: Verified
Quanlong Huang: Looks good to me, but someone else must approve
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 2
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR](2.x) 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/12504 )
Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................
Patch Set 1:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3787/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 03:30:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) IMPALA-7174: fix test cancellation for RELEASE builds
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/12504 )
Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................
Patch Set 1: Code-Review+1
Tim, would you like to have a look at this? I resolve the minor conflicts and it looks good to me.
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 08:03:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR](2.x) 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/12504 )
Change subject: IMPALA-7174: fix test_cancellation for RELEASE builds
......................................................................
Patch Set 1: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/12504
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: 2.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I41da7b5ac58a468a8ed117777969906f63df6d4b
Gerrit-Change-Number: 12504
Gerrit-PatchSet: 1
Gerrit-Owner: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Feb 2019 07:29:09 +0000
Gerrit-HasComments: No