You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org> on 2018/10/12 17:59:11 UTC
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11669
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
IMPALA-7545: Add queueing reason to query log
After this change, the HS2 GetLog() function returns the queueing
reason for a query when it is queued by the AdmissionController.
Testing: Added an end-to-end test to test_admission_controller.py
to verify the query logs returned.
Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/service/impala-hs2-server.cc
M tests/custom_cluster/test_admission_controller.py
4 files changed, 78 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11669/1
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 01:47:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Oct 2018 22:01:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 7:
I don't love the approach of using profile strings as an interface between modules - it feels like this added a little bit of technical debt, but I appreciate that you added tests and shared the constant definitions so this should at least be maintainable.
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 22 Oct 2018 15:23:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1041/ : 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/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 22:20:46 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Oct 2018 17:49:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
IMPALA-7545: Add queueing reason to query log
After this change, the HS2 GetLog() function returns the queueing
reason for a query when it is queued by the AdmissionController.
Testing: Added an end-to-end test to test_admission_controller.py
to verify the query logs returned.
Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/service/impala-hs2-server.cc
M tests/custom_cluster/test_admission_controller.py
4 files changed, 88 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11669/4
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
IMPALA-7545: Add queueing reason to query log
After this change, the HS2 GetLog() function returns the queueing
reason for a query when it is queued by the AdmissionController.
Testing: Added an end-to-end test to test_admission_controller.py
to verify the query logs returned.
Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/service/impala-hs2-server.cc
M tests/custom_cluster/test_admission_controller.py
4 files changed, 81 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11669/3
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 4:
(3 comments)
just a few nits left
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@845
PS4, Line 845: queueing
nit: queuing
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@849
PS4, Line 849: assert re.search("Admission result : Queued\nLatest admission queue reason "
also add assert for "Admission result: Queued"
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@849
PS4, Line 849: re.search("Admission result : Queued\nLatest admission queue reason "
: ": number of running queries 1 is at or over limit 1\n", log)
just use "string_to_search" in str instead of a regex search. no need for extra overhead
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Oct 2018 00:30:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1047/ : 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/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Sat, 13 Oct 2018 00:45:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 3:
(7 comments)
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@822
PS3, Line 822: , max_queued=1,
: pool_max_mem=10 * PROC_MEM_TEST_LIMIT,
: queue_wait_timeout_ms=2 * STATESTORE_RPC_FREQUENCY_MS
you can get rid of these
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@834
PS3, Line 834: long_running_query
nit: just write the query directly here
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@836
PS3, Line 836: HS2TestSuite.check_response(long_query_resp)
use self.hs2_client.wait_for_admission_control() to make sure the first query has started running
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@837
PS3, Line 837: # Wait for statestore update to update the query admitted in each node.
: sleep(STATESTORE_RPC_FREQUENCY_MS / 1000)
you dont need this, the query will anyways run on a single node
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@843
PS3, Line 843: query
nit: write the query directly here
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@846
PS3, Line 846: HS2TestSuite.check_response(execute_statement_resp
use self.hs2_client.wait_for_operation_state() and wait for PENDING state to make sure the second query has been queued
http://gerrit.cloudera.org:8080/#/c/11669/3/tests/custom_cluster/test_admission_controller.py@852
PS3, Line 852:
close the running queries at the end
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 22:15:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1038/ : 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/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 18:33:03 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc@837
PS1, Line 837: const string* queued_reason = req
> there is a small window where this can be null, its better to just check fo
Done
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc@838
PS1, Line 838: issionControll
> maybe just use PROFILE_INFO_KEY_LAST_QUEUED_REASON here instead.
Done
http://gerrit.cloudera.org:8080/#/c/11669/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/11669/1/tests/custom_cluster/test_admission_controller.py@820
PS1, Line 820: @pytest.mark.execute_serially
> You can setup a query to queue pretty easily instead of doing this:
Done
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 21:48:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
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/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
IMPALA-7545: Add queuing reason to query log
After this change, the HS2 GetLog() function returns the queuing
reason for a query when it is queued by the AdmissionController.
Testing: Added an end-to-end test to test_admission_controller.py
to verify the query logs returned.
Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Reviewed-on: http://gerrit.cloudera.org:8080/11669
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/scheduling/admission-controller.h
M be/src/service/impala-hs2-server.cc
M tests/custom_cluster/test_admission_controller.py
4 files changed, 89 insertions(+), 13 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@845
PS4, Line 845: queuing
> nit: queuing
Done
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@849
PS4, Line 849: assert "Admission result : Queued" in log, log
> also add assert for "Admission result: Queued"
Done
http://gerrit.cloudera.org:8080/#/c/11669/4/tests/custom_cluster/test_admission_controller.py@849
PS4, Line 849: "Admission result : Queued" in log, log
: assert "Latest admission queue reason : number of running queries 1 i
> just use "string_to_search" in str instead of a regex search. no need for e
Done
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Oct 2018 16:57:46 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3317/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Mon, 15 Oct 2018 22:01:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7545: Add queueing reason to query log
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queueing reason to query log
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc@837
PS1, Line 837: DCHECK(queued_reason != nullptr);
there is a small window where this can be null, its better to just check for it instead of a dcheck
http://gerrit.cloudera.org:8080/#/c/11669/1/be/src/service/impala-hs2-server.cc@838
PS1, Line 838: Queued reason:
maybe just use PROFILE_INFO_KEY_LAST_QUEUED_REASON here instead.
Also worth printing PROFILE_INFO_KEY_ADMISSION_RESULT : PROFILE_INFO_VAL_QUEUED so that the total output looks like:
Admission result: Queued
Latest admission queue reason: number of running queries 1 is at or over limit 1
http://gerrit.cloudera.org:8080/#/c/11669/1/tests/custom_cluster/test_admission_controller.py
File tests/custom_cluster/test_admission_controller.py:
http://gerrit.cloudera.org:8080/#/c/11669/1/tests/custom_cluster/test_admission_controller.py@820
PS1, Line 820: @pytest.mark.execute_serially
You can setup a query to queue pretty easily instead of doing this:
start a cluster with -default_pool_max_requests 1 , start a long running query like sleep(100000), then the next query you try to run will be queued
For reference, see L759-764
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 1
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 20:57:21 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7545: Add queuing reason to query log
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11669 )
Change subject: IMPALA-7545: Add queuing reason to query log
......................................................................
IMPALA-7545: Add queuing reason to query log
After this change, the HS2 GetLog() function returns the queuing
reason for a query when it is queued by the AdmissionController.
Testing: Added an end-to-end test to test_admission_controller.py
to verify the query logs returned.
Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
---
M be/src/scheduling/admission-controller.cc
M be/src/scheduling/admission-controller.h
M be/src/service/impala-hs2-server.cc
M tests/custom_cluster/test_admission_controller.py
4 files changed, 89 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/11669/5
--
To view, visit http://gerrit.cloudera.org:8080/11669
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I2e5d8de4f6691a9ba2594ca68c54ea4dca760545
Gerrit-Change-Number: 11669
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>