[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 8: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4890/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 05:25:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#8). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoiding unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- 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 A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,058 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/8 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8921: User short name for Ranger grant/revoke requests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14185 ) Change subject: IMPALA-8921: User short name for Ranger grant/revoke requests .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4483/ : 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/14185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 Gerrit-Change-Number: 14185 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 06 Sep 2019 04:57:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14181 ) Change subject: IMPALA-1071: Distributable python package for impala-shell .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 Gerrit-Change-Number: 14181 Gerrit-PatchSet: 3 Gerrit-Owner: David Knupp Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Dinesh Garg (430) Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 06 Sep 2019 04:26:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8921: User short name for Ranger grant/revoke requests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14185 ) Change subject: IMPALA-8921: User short name for Ranger grant/revoke requests .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4889/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 Gerrit-Change-Number: 14185 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 06 Sep 2019 04:19:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8921: User short name for Ranger grant/revoke requests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14185 ) Change subject: IMPALA-8921: User short name for Ranger grant/revoke requests .. Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/14185/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/14185/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@126 PS1, Line 126: new User(header.getRequesting_user()).getShortName(), true, params.getPrincipal_name(), line too long (95 > 90) http://gerrit.cloudera.org:8080/#/c/14185/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@138 PS1, Line 138: new User(header.getRequesting_user()).getShortName(), false, params.getPrincipal_name(), line too long (96 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 Gerrit-Change-Number: 14185 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Fri, 06 Sep 2019 04:18:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8921: User short name for Ranger grant/revoke requests
Bharath Vissapragada has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14185 Change subject: IMPALA-8921: User short name for Ranger grant/revoke requests .. IMPALA-8921: User short name for Ranger grant/revoke requests For certain grant/revoke Ranger commmands, we ended up passing the full name which is a problem when kerberos is enabled. Ranger expects the short name during authorization. Testing: We do not have test coverage with kerberos enabled, so I inspected the code manually to make sure we are using getShortName() everywhere. Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 --- M common/thrift/CatalogService.thrift M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java 2 files changed, 6 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/85/14185/1 -- To view, visit http://gerrit.cloudera.org:8080/14185 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3dc1bf55d50dc2e5fa6e07f16644f0a2773f2d23 Gerrit-Change-Number: 14185 Gerrit-PatchSet: 1 Gerrit-Owner: Bharath Vissapragada
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 7: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4888/ -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 04:05:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8917: Remove hostname from webui links if Knox isn't being used
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14184 ) Change subject: IMPALA-8917: Remove hostname from webui links if Knox isn't being used .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4482/ : 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/14184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ca88115ef4a0fe2d68f1e9ee5fcc4bb415a2d85 Gerrit-Change-Number: 14184 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 06 Sep 2019 03:47:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8917: Remove hostname from webui links if Knox isn't being used
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14184 ) Change subject: IMPALA-8917: Remove hostname from webui links if Knox isn't being used .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/14184/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/14184/1/tests/webserver/test_web_pages.py@163 PS1, Line 163: r flake8: E501 line too long (95 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/14184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ca88115ef4a0fe2d68f1e9ee5fcc4bb415a2d85 Gerrit-Change-Number: 14184 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 06 Sep 2019 03:10:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8917: Remove hostname from webui links if Knox isn't being used
Thomas Tauber-Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14184 Change subject: IMPALA-8917: Remove hostname from webui links if Knox isn't being used .. IMPALA-8917: Remove hostname from webui links if Knox isn't being used IMPALA-8897 added the hostname to all links on the debug webui in order to facilitate proxying connections through Apache Knox. This makes the webui difficult to use in situations where the hostname is not DNS-resolvable. This patch fixes this by only including the hostname with links if Knox proxying is actually being used, which we determine by looking for the 'host' parameter in the request, which is used by Knox to determine what impalad host to connect to. It removes the hidden form fields that were added to support Knox integration when not being accessed through Knox. It also adds a class comment on Webserver explaining the requirements for keeping the webui compatible with Knox. Testing: - Added a test that checks that links on the webui are made absolute when the 'host' parameter is in the request. Change-Id: I0ca88115ef4a0fe2d68f1e9ee5fcc4bb415a2d85 --- M be/src/util/webserver.cc M be/src/util/webserver.h M tests/webserver/test_web_pages.py M www/form-hidden-inputs.tmpl 4 files changed, 48 insertions(+), 19 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/14184/1 -- To view, visit http://gerrit.cloudera.org:8080/14184 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I0ca88115ef4a0fe2d68f1e9ee5fcc4bb415a2d85 Gerrit-Change-Number: 14184 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4481/ : 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/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 01:08:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4888/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 00:32:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4480/ : 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/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 00:32:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#7). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoiding unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- 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 A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,060 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/7 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 6: (1 comment) http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py@646 PS6, Line 646: i > flake8: F821 undefined name 'i' aargh, vim troubles, extraneous character. -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Fri, 06 Sep 2019 00:27:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14181 ) Change subject: IMPALA-1071: Distributable python package for impala-shell .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4887/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 Gerrit-Change-Number: 14181 Gerrit-PatchSet: 3 Gerrit-Owner: David Knupp Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Dinesh Garg (430) Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Fri, 06 Sep 2019 00:21:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4479/ : 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/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 23:58:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14143 )
Change subject: IMPALA-8572: Log query events before unregister.
..
Patch Set 6:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py
File tests/common/impala_test_suite.py:
http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py@646
PS6, Line 646: i
flake8: E225 missing whitespace around operator
http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py@646
PS6, Line 646: i
flake8: F821 undefined name 'i'
http://gerrit.cloudera.org:8080/#/c/14143/6/tests/common/impala_test_suite.py@646
PS6, Line 646: \
flake8: E211 whitespace before '('
--
To view, visit http://gerrit.cloudera.org:8080/14143
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e
Gerrit-Change-Number: 14143
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Reviewer: radford nguyen
Gerrit-Comment-Date: Thu, 05 Sep 2019 23:52:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#6). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoiding unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- 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 A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,061 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/6 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries
Bikramjeet Vig has posted comments on this change. (
http://gerrit.cloudera.org:8080/14183 )
Change subject: IMPALA-8830: Fix executor group assignment of coordinator only
queries
..
Patch Set 1:
(5 comments)
> (5 comments)
>
> I think it would be good to unify the ways in which we pass the
> local BE desc into the scheduler. With this change we pass it both
> as a BE desc and as the coordinator-only group. We could stay with
> the old approach and pass an ExecutorConfig with an empty executor
> group, and then build the coordinator-only group as we did before
> in the scheduler. I don't see an easy way to pass it with a
> coordinator-only group when also using a regular executor group for
> scheduling.
We cant pass an executor group because further down the code path we expect an
exec group associated with an admitted schedule to have non-zero executors as
we use the group size for making admission decisions.
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h@93
PS1, Line 93: ExecutorGroup local_coord_only_group =
ExecutorGroup("coordinator-only-group");
> It seems that so far we have created a group with the coordinator on-demand
I thought of that but decided against it since the whole premise of an executor
group is that it is composed of executors only, which is why i called this a
"pseudo group". Also adding a special group in the ExecutorGroups list means
there will always be a healthy group with ""coordinator-only-group"" being a
reserved group name (not sure how we can enforce that). moreover, this group
wont be bound by the group_name->resource_pool mapping rule. Basically a bunch
of special handling will be required for that special exec group and having
that implicit in a regular vector of ExecutorGroup objects would be weird.
I initially thought of creating ExecutorGroups class that encompasses these
special cases, and abstract away some of that logic there but if I keep going
that route I would end up with something like the all encompassing Snapshot
struct, so I just added the pseudo group to it.
I agree all this is a bit messy so open to suggestions. Maybe we can just pass
the full snapshot and a list of exec groups for the resource pool to the
scheduler and handle some of the messiness there (handling the
IsCoordinatorOnlyQuery, temp-coordinator-only-group parts,
Scheduler::ExecutorConfig parts).
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@143
PS1, Line 143: bool exec_at_coord = (fragment.partition.type ==
TPartitionType::UNPARTITIONED);
> Should exec_at_coord now be replaced with IsCoordinatorOnlyQuery()?
the exec_at_coord used here is at the fragment level to identify if a fragment
is to be scheduled on the coordinator, whereas IsCoordinatorOnlyQuery is for
the whole query that also makes sure the plan contains only one fragment.
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@465
PS1, Line 465: ExecutorGroup
coord_only_executor_group("temp-coordinator-only-group");
> This should now use the coordinator-only group, no?
Unfortunately, the way scheduling works, this temp group is required because
ComputeScanRangeAssignment is done for all queries and "non-coordinator only"
queries would not have coordinator-only-group in their ExecutorConfig
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: schduled
> nit: typo
will fix in the next patch
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: ideally
> Why "ideally" instead of "that gets scheduled".
will fix in the next patch
--
To view, visit http://gerrit.cloudera.org:8080/14183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Thu, 05 Sep 2019 23:45:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 4: (6 comments) http://gerrit.cloudera.org:8080/#/c/14143/4//COMMIT_MSG Commit Message: PS4: > 1) the main reason to change the behaviour in this patch would be to simpli Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@335 PS5, Line 335: ; > flake8: E703 statement ends with a semicolon Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@646 PS5, Line 646: ) > flake8: E501 line too long (91 > 90 characters) Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py File tests/common/test_result_verifier.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@22 PS5, Line 22: import json > flake8: F401 'json' imported but unused Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@631 PS5, Line 631: def verify_lineage(expected, actual, lineage_skip_json_keys=DEFAULT_LINEAGE_SKIP_KEYS): > flake8: E302 expected 2 blank lines, found 1 Done http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@635 PS5, Line 635: p > flake8: E501 line too long (102 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 23:44:28 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before unregister. .. Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py File tests/common/impala_test_suite.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@335 PS5, Line 335: ; flake8: E703 statement ends with a semicolon http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/impala_test_suite.py@646 PS5, Line 646: ) flake8: E501 line too long (91 > 90 characters) http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py File tests/common/test_result_verifier.py: http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@22 PS5, Line 22: import json flake8: F401 'json' imported but unused http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@631 PS5, Line 631: def verify_lineage(expected, actual, lineage_skip_json_keys=DEFAULT_LINEAGE_SKIP_KEYS): flake8: E302 expected 2 blank lines, found 1 http://gerrit.cloudera.org:8080/#/c/14143/5/tests/common/test_result_verifier.py@635 PS5, Line 635: p flake8: E501 line too long (102 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 23:39:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8572: Log query events before unregister.
Hello radford nguyen, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14143 to look at the new patch set (#5). Change subject: IMPALA-8572: Log query events before unregister. .. IMPALA-8572: Log query events before unregister. Currently, the query events (audits and lineages) are logged as a part of query unregistration. This delays the event logging in cases where the Unregister() is delayed by client for some reason (ex: Hue does not call Unregister until the browser tab is closed) or the client goes away without calling Unregister and the query timeout kicks in. This patch moves this event logging to an earlier stage in the query lifecycle. Moved the event logging related code into ClientRequestState for easier code refactoring. The conditions under which the events are logged are slightly modified by this patch. Without the patch, events are logged for unsuccessful queries if atleast a single fetch is perfomed. This patch relaxes this guarantee to log events for any query that reaches the FINISHED state (rows are available to fetch by the client) and does not wait for a fetch to be performed. This simplifies the coordinator state machine by avoid unnecessary synchronization. Added some test coverage for coordinator side code paths for writing lineages. fe specific lineage tests only verified the correctness of lineage created but did not test whether it was being flushed correctly to the disk. Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e --- 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 A testdata/workloads/functional-query/queries/QueryTest/lineage.test M tests/common/impala_test_suite.py M tests/common/test_result_verifier.py M tests/custom_cluster/test_lineage.py M tests/unittests/test_file_parser.py M tests/util/test_file_parser.py 10 files changed, 6,059 insertions(+), 239 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/43/14143/5 -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14181 ) Change subject: IMPALA-1071: Distributable python package for impala-shell .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4478/ : 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/14181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 Gerrit-Change-Number: 14181 Gerrit-PatchSet: 3 Gerrit-Owner: David Knupp Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Dinesh Garg (430) Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 23:39:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
Hello Bharath Vissapragada, Greg Rahn, Lars Volker, David Rorke, Tim Armstrong, Dinesh Garg, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14181 to look at the new patch set (#3). Change subject: IMPALA-1071: Distributable python package for impala-shell .. IMPALA-1071: Distributable python package for impala-shell The patch adds a set of scripts for converting the impala-shell into a true distributable python package. The package can be installed using familiar python commands, e.g.: $ python setup.py (install|develop) or $ pip install -e /path/to/dist/dir The entry point script, make_python_package.sh, will run as a part of the standard sequence of steps that results from calling buildall.sh, and will produce a gzipped tarball inside of Impala/shell/dist as an artifact. Thereafter, make_python_package.sh can be run manually any time. The expectation is that an official maintainer would need to manually upload official releases to the Python Package Index as appropriate. Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 --- M bin/rat_exclude_files.txt M shell/impala_client.py M shell/make_shell_tarball.sh A shell/packaging/MANIFEST.in A shell/packaging/README.md A shell/packaging/__init__.py A shell/packaging/make_python_package.sh A shell/packaging/requirements.txt A shell/packaging/setup.py 9 files changed, 397 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/14181/3 -- To view, visit http://gerrit.cloudera.org:8080/14181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 Gerrit-Change-Number: 14181 Gerrit-PatchSet: 3 Gerrit-Owner: David Knupp Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: David Knupp Gerrit-Reviewer: David Rorke Gerrit-Reviewer: Dinesh Garg (430) Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/14183 )
Change subject: IMPALA-8830: Fix executor group assignment of coordinator only
queries
..
Patch Set 1:
(5 comments)
I think it would be good to unify the ways in which we pass the local BE desc
into the scheduler. With this change we pass it both as a BE desc and as the
coordinator-only group. We could stay with the old approach and pass an
ExecutorConfig with an empty executor group, and then build the
coordinator-only group as we did before in the scheduler. I don't see an easy
way to pass it with a coordinator-only group when also using a regular executor
group for scheduling.
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/cluster-membership-mgr.h@93
PS1, Line 93: ExecutorGroup local_coord_only_group =
ExecutorGroup("coordinator-only-group");
It seems that so far we have created a group with the coordinator on-demand in
the scheduler. Having a special group inside executor_groups could be a cleaner
approach. Can we move this group there?
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc
File be/src/scheduling/scheduler.cc:
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@143
PS1, Line 143: bool exec_at_coord = (fragment.partition.type ==
TPartitionType::UNPARTITIONED);
Should exec_at_coord now be replaced with IsCoordinatorOnlyQuery()?
http://gerrit.cloudera.org:8080/#/c/14183/1/be/src/scheduling/scheduler.cc@465
PS1, Line 465: ExecutorGroup
coord_only_executor_group("temp-coordinator-only-group");
This should now use the coordinator-only group, no?
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py
File tests/custom_cluster/test_executor_groups.py:
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: schduled
nit: typo
http://gerrit.cloudera.org:8080/#/c/14183/1/tests/custom_cluster/test_executor_groups.py@29
PS1, Line 29: ideally
Why "ideally" instead of "that gets scheduled".
--
To view, visit http://gerrit.cloudera.org:8080/14183
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5
Gerrit-Change-Number: 14183
Gerrit-PatchSet: 1
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Thu, 05 Sep 2019 22:54:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. IMPALA-8897 (part 2): Fix javascript to work with Knox integration This patch qualifies all urls accessed via javascript in the webui with the appropriate host:port in order to allow these urls to work when the connection to the webui is being proxied through Apache Knox. It accomplishes this with the function make_url(), which takes the href of a link from the page, which may have been rewritten by Knox, and appends a path to it. This patch also fixes on issue on the /admissions page, where resetting a pool's stats could fail due to the page being reloaded before the reset is executed. Fixed by moving the call to reload to the completion callback for the 'reset' endpoint. Testing: - Added a regex to test_knox_compatability that performs a rough check for places in the tmpl files where urls are used in javascript without calling make_url(). Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Reviewed-on: http://gerrit.cloudera.org:8080/14173 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M tests/webserver/test_web_pages.py M www/admission_controller.tmpl M www/common-header.tmpl M www/query_plan.tmpl M www/query_summary.tmpl M www/rpcz.tmpl 6 files changed, 38 insertions(+), 13 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 4 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 3: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 22:52:04 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix statestored /rpcz page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14175 ) Change subject: Fix statestored /rpcz page .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf01c3af219e17728836c63b1e0ff10cbcb221ff Gerrit-Change-Number: 14175 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 22:50:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14183 ) Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4477/ : 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/14183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5 Gerrit-Change-Number: 14183 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Thu, 05 Sep 2019 22:37:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8858: Add observability of the query load on executor groups
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/14103 )
Change subject: IMPALA-8858: Add observability of the query load on executor
groups
..
Patch Set 8:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/runtime/exec-env.cc
File be/src/runtime/exec-env.cc:
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/runtime/exec-env.cc@364
PS8, Line 364:
: TUpdateExecutorMembershipRequest update_req;
: for (const auto& it : snapshot->current_backends) {
: const TBackendDescriptor& backend = it.second;
: if (backend.is_executor) {
:
update_req.hostnames.insert(backend.address.hostname);
: update_req.ip_addresses.insert(backend.ip_address);
: update_req.num_executors++;
: }
: }
: Status status =
this->frontend()->UpdateExecutorMembership(update_req);
: if (!status.ok()) {
: LOG(WARNING) << "Error updating frontend membership
snapshot: "
:<< status.GetDetail();
: }
I'm inclined to think that this code should not be in the ExecEnv (but we can
postpone that to a cleanup change). I'm aware that the typedefs inside the CMM
cannot be forward-declared and we shouldn't include it in the frontend, but we
should look into pulling the typedefs out of the CMM and then simplify this
code by moving it into the destination of the callbacks.
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/runtime/exec-env.cc@494
PS8, Line 494: std::
nit: remove std:: in .cc files
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/runtime/exec-env.cc@496
PS8, Line 496: current_backend_set.insert(it.second.address);
Same as comment in the other callback
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/admission-controller.h@948
PS8, Line 948: load metric of the 'grp_name' executor group by 'delta'
While reading this I was confused what 'delta' is, maybe instead of "query
load" we should use "query count" (load is often a value between 0 and 1)?
'num_queries' might also be a good parameter name if we rename the method to
something like "IncExecGroupMetricLocked()".
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.h
File be/src/scheduling/cluster-membership-mgr.h:
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.h@124
PS8, Line 124: typedef std::function UpdateCallbackFn;
Previously, UpdateLocalServerFn was only called when backends had been deleted.
I think it would be nice to preserve that, e.g. by adding a bool
can_contain_deletes (feel free to find a better name) to the callback.
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.cc
File be/src/scheduling/cluster-membership-mgr.cc:
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.cc@321
PS8, Line 321: SetStateAndUpdateMetrics(new_state);
I know I asked for the metric update to be moved there, but now that we
generalized the callback mechanism, I think it could be nice to update the
metrics in a callback. What do you think?
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.cc@322
PS8, Line 322:
nit: extra space
http://gerrit.cloudera.org:8080/#/c/14103/8/be/src/scheduling/cluster-membership-mgr.cc@323
PS8, Line 323: NotifyAllCallbackUpdateFns
I think NotifyListeners(new_state) might be more concise but clear enough
http://gerrit.cloudera.org:8080/#/c/14103/6/common/thrift/metrics.json
File common/thrift/metrics.json:
http://gerrit.cloudera.org:8080/#/c/14103/6/common/thrift/metrics.json@2502
PS6, Line 2502: ad.$0"
> do you think num-running-queries is more appropriate here?
maybe even num-running is ok (see my comment elsewhere about "load"). How do we
call other metrics that track number of queries?
--
To view, visit http://gerrit.cloudera.org:8080/14103
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I58cde8699c33af8b87273437e9d8bf6371a34539
Gerrit-Change-Number: 14103
Gerrit-PatchSet: 8
Gerrit-Owner: Bikramjeet Vig
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Comment-Date: Thu, 05 Sep 2019 21:59:43 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8830: Fix executor group assignment of coordinator only queries
Bikramjeet Vig has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14183 Change subject: IMPALA-8830: Fix executor group assignment of coordinator only queries .. IMPALA-8830: Fix executor group assignment of coordinator only queries With this fix, coordinator only queries are submitted to a pseudo executor group that only contains the local coordinator to which the query was submitted. This allows running coordinator only queries regardless of the presence of any healthy executor groups. Testing: Added a custom cluster test and modified tests that relied on coordinator only queries to be queued in absence of executor groups. Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5 --- M be/src/scheduling/admission-controller.cc M be/src/scheduling/cluster-membership-mgr.cc M be/src/scheduling/cluster-membership-mgr.h M be/src/scheduling/executor-group.cc M be/src/scheduling/scheduler.cc M be/src/scheduling/scheduler.h M tests/custom_cluster/test_executor_groups.py 7 files changed, 55 insertions(+), 27 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/83/14183/1 -- To view, visit http://gerrit.cloudera.org:8080/14183 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I8fe098032744aa20bbbe4faddfc67e7a46ce03d5 Gerrit-Change-Number: 14183 Gerrit-PatchSet: 1 Gerrit-Owner: Bikramjeet Vig
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 19: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4884/ -- To view, visit http://gerrit.cloudera.org:8080/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 19 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 21:39:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
David Knupp has posted comments on this change. (
http://gerrit.cloudera.org:8080/14181 )
Change subject: IMPALA-1071: Distributable python package for impala-shell
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14181/2/shell/make_shell_tarball.sh
File shell/make_shell_tarball.sh:
http://gerrit.cloudera.org:8080/#/c/14181/2/shell/make_shell_tarball.sh@135
PS2, Line 135: DIST_DIR="${SHELL_HOME}"/dist CLEAN_DIST="true"
"${SHELL_HOME}"/packaging/make_python_package.sh
> line too long (96 > 90)
Ack
--
To view, visit http://gerrit.cloudera.org:8080/14181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044
Gerrit-Change-Number: 14181
Gerrit-PatchSet: 2
Gerrit-Owner: David Knupp
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: David Knupp
Gerrit-Reviewer: David Rorke
Gerrit-Reviewer: Dinesh Garg (430)
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 05 Sep 2019 21:28:27 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14181 )
Change subject: IMPALA-1071: Distributable python package for impala-shell
..
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14181/2/shell/make_shell_tarball.sh
File shell/make_shell_tarball.sh:
http://gerrit.cloudera.org:8080/#/c/14181/2/shell/make_shell_tarball.sh@135
PS2, Line 135: DIST_DIR="${SHELL_HOME}"/dist CLEAN_DIST="true"
"${SHELL_HOME}"/packaging/make_python_package.sh
line too long (96 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14181
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044
Gerrit-Change-Number: 14181
Gerrit-PatchSet: 2
Gerrit-Owner: David Knupp
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Thu, 05 Sep 2019 21:23:23 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
David Knupp has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14181 Change subject: IMPALA-1071: Distributable python package for impala-shell .. IMPALA-1071: Distributable python package for impala-shell The patch adds a set of scripts for converting the impala-shell into a true distributable python package. The package can be installed using familiar python commands, e.g.: $ python setup.py (install|develop) or $ pip install -e /path/to/dist/dir The entry point script, make_python_package.sh, will run as a part of the standard sequence of steps that results from calling buildall.sh, and will produce a gzipped tarball inside of Impala/shell/dist as an artifact. Thereafter, make_python_package.sh can be run manually any time. The expectation is that an official maintainer would need to manually upload official releases to the Python Package Index as appropriate. Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 --- M shell/impala_client.py M shell/make_shell_tarball.sh A shell/packaging/MANIFEST.in A shell/packaging/README.md A shell/packaging/__init__.py A shell/packaging/make_python_package.sh A shell/packaging/requirements.txt A shell/packaging/setup.py 8 files changed, 392 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/81/14181/2 -- To view, visit http://gerrit.cloudera.org:8080/14181 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ib8c745bf6a16f0c039430152745a2f00e044 Gerrit-Change-Number: 14181 Gerrit-PatchSet: 2 Gerrit-Owner: David Knupp
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4475/ : 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/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 21:20:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 9: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 21:17:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4474/ : 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/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 21:09:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 9: (2 comments) http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc File be/src/runtime/coordinator-backend-resource-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@95 PS7, Line 95: euristics. > Not sure I follow. Can a query run without a coordinator fragment? DML queries won't have a coordinator fragment (there's a coordinator, but no fragment returning rows). http://gerrit.cloudera.org:8080/#/c/14104/6/be/src/runtime/coordinator.cc File be/src/runtime/coordinator.cc: http://gerrit.cloudera.org:8080/#/c/14104/6/be/src/runtime/coordinator.cc@773 PS6, Line 773: backend_released_barrier_.Notify(); > Thought about it a bit, think there are some subtle race conditions in Coun Makes sense -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 20:53:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-1071: Distributable python package for impala-shell
David Knupp has abandoned this change. ( http://gerrit.cloudera.org:8080/9771 ) Change subject: IMPALA-1071: Distributable python package for impala-shell .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/9771 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I0422e0c95101400f841ba407a7bd1b2b12d363e0 Gerrit-Change-Number: 9771 Gerrit-PatchSet: 4 Gerrit-Owner: David Knupp Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Greg Rahn Gerrit-Reviewer: Jim Apple Gerrit-Reviewer: Michael Brown Gerrit-Reviewer: Nithya Janarthanan Gerrit-Reviewer: Philip Zeyliger Gerrit-Reviewer: Tianyi Wang
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/14104/8/tests/custom_cluster/test_result_spooling.py File tests/custom_cluster/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/14104/8/tests/custom_cluster/test_result_spooling.py@66 PS8, Line 66: s > flake8: F821 undefined name 'sleep' Done -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 20:37:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Hello Michael Ho, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14104 to look at the new patch set (#9). Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. IMPALA-8803: Coordinator should release admitted memory per-backend Changes the Coordinator to release admitted memory when each Backend completes, rather than waiting for the entire query to complete before releasing admitted memory. When the Coordinator detects that a Backend has completed (via ControlService::ReportExecStatus) it updates the state of the Backend in Coordinator::BackendResourceState. BackendResourceState tracks the state of the admitted resources for each Backend and decides when the resources for a group of Backend states should be released. BackendResourceState defines a state machine to help coordinate the state of the admitted memory for each Backend. It guarantees that by the time the query is shutdown, all Backends release their admitted memory. BackendResourceState implements three rules to control the rate at which the Coordinator releases admitted memory from the AdmissionController: * Resources are released at most once every 1 second, this prevents short lived queries from causing high load on the admission controller * Resources are released at most O(log(num_backends)) times; the BackendResourceStates can release multiple BackendStates from the AdmissionController at a time * All pending resources are released if the only remaining Backend is the Coordinator Backend; this is useful for result spooling where all Backends may complete, except for the Coordinator Backend Exposes the following hidden startup flags to help tune the heuristics above: --batched_release_decay_factor * Defaults to 2 * Controls the base value for the O(log(num_backends)) bound when batching the release of Backends. --release_backend_states_delay_ms * Defaults to 1000 milliseconds * Controls how often Backends can release their resources. Testing: * Ran core tests * Added new tests to test_result_spooling.py and test_admission_controller.py Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 --- M be/src/runtime/CMakeLists.txt A be/src/runtime/coordinator-backend-resource-state.cc A be/src/runtime/coordinator-backend-state-test.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_auto_scaling.py M tests/custom_cluster/test_executor_groups.py A tests/custom_cluster/test_result_spooling.py M tests/util/auto_scaler.py A tests/util/web_pages_util.py 16 files changed, 1,044 insertions(+), 94 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/14104/9 -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 9 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/14104/8/tests/custom_cluster/test_result_spooling.py File tests/custom_cluster/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/14104/8/tests/custom_cluster/test_result_spooling.py@66 PS8, Line 66: s flake8: F821 undefined name 'sleep' -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 20:28:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Hello Michael Ho, Tim Armstrong, Bikramjeet Vig, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14104 to look at the new patch set (#8). Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. IMPALA-8803: Coordinator should release admitted memory per-backend Changes the Coordinator to release admitted memory when each Backend completes, rather than waiting for the entire query to complete before releasing admitted memory. When the Coordinator detects that a Backend has completed (via ControlService::ReportExecStatus) it updates the state of the Backend in Coordinator::BackendResourceState. BackendResourceState tracks the state of the admitted resources for each Backend and decides when the resources for a group of Backend states should be released. BackendResourceState defines a state machine to help coordinate the state of the admitted memory for each Backend. It guarantees that by the time the query is shutdown, all Backends release their admitted memory. BackendResourceState implements three rules to control the rate at which the Coordinator releases admitted memory from the AdmissionController: * Resources are released at most once every 1 second, this prevents short lived queries from causing high load on the admission controller * Resources are released at most O(log(num_backends)) times; the BackendResourceStates can release multiple BackendStates from the AdmissionController at a time * All pending resources are released if the only remaining Backend is the Coordinator Backend; this is useful for result spooling where all Backends may complete, except for the Coordinator Backend Exposes the following hidden startup flags to help tune the heuristics above: --batched_release_decay_factor * Defaults to 2 * Controls the base value for the O(log(num_backends)) bound when batching the release of Backends. --release_backend_states_delay_ms * Defaults to 1000 milliseconds * Controls how often Backends can release their resources. Testing: * Ran core tests * Added new tests to test_result_spooling.py and test_admission_controller.py Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 --- M be/src/runtime/CMakeLists.txt A be/src/runtime/coordinator-backend-resource-state.cc A be/src/runtime/coordinator-backend-state-test.cc M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/coordinator-backend-state.h M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M tests/custom_cluster/test_admission_controller.py M tests/custom_cluster/test_auto_scaling.py M tests/custom_cluster/test_executor_groups.py A tests/custom_cluster/test_result_spooling.py M tests/util/auto_scaler.py A tests/util/web_pages_util.py 16 files changed, 1,043 insertions(+), 94 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/04/14104/8 -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 8 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 7: (15 comments) http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc File be/src/runtime/coordinator-backend-resource-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@81 PS7, Line 81: num_pending_releasable_or_released_ > Would it be simpler to track the negation (i.e. num_in_use_) instead ? We o Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@95 PS7, Line 95: is_coordinator_the_last_unreleased_backend > If I understand it correctly, there is no adverse effect for this heuristic Not sure I follow. Can a query run without a coordinator fragment? If there is no coordinator fragment, then this heuristic will always be false since there is no BackendState where 'is_coord_backend' is true. Although this won't affect the other heuristics. Worse case, that just means releasing admission control resources is delayed until the query is closed, which is what the current code does anyway. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@106 PS7, Line 106: } > DCHECK_GE(num_pending_, 0); Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@117 PS7, Line 117: DCHECK( > DCHECK_NE Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@457 PS7, Line 457: PENDING state > or RELESABLE ? I've revised the method description to make things a bit clearer. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@460 PS7, Line 460: /// released. > A no-op if the BackendResourceState is closed already. Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@465 PS7, Line 465: /// BackendStates have been released. > This may be called even after CloseAndGetUnreleasedBackends(). Yes, revised the method comment. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@509 PS7, Line 509: BackendResourceState is closed, > May help to clarify what may and may not happen after transitioning to the I've updated the method comments above, the class comment also has some notes about this. Let me know if you think those are sufficient, otherwise I can add some more. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h@851 PS7, Line 851: specificied > typo Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h@852 PS7, Line 852: UpdateStatsForHost( > nit: This name is a bit too similar to UpdateHostStats() and they seem to s Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@408 PS7, Line 408: PrintId(schedule.query_id())); > Should we add a continue; in this case ? Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@406 PS7, Line 406: DCHECK(false) << strings::Substitute( : "Error: Cannot find host address $0 for query $1.", PrintThrift(host_addr), : PrintId(schedule.query_id())); > Shouldn't this be LOG(ERROR) or something in addition to DCHECK(false); ? Done http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@1531 PS7, Line 1531: } > DCHECK_EQ(num_release_backends_.find(schedule->query_id()), num_release_bac Done http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_executor_groups.py File tests/custom_cluster/test_executor_groups.py: http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_executor_groups.py@236 PS7, Line 236: 3 > Why this change ? This test started failing because they assume a max number of queries can be run within an executor group and enforce this by setting -max_concurrent_queries. The issue is that they only set -max_concurrent_queries for executors, and set a much higher value for coordinators. That causes problems with this patch because backends might be released independently (e.g. executor backends might be released first, and then coordinator backends). This breaks the assertions in
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4473/ : 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/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 19:10:50 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix statestored /rpcz page
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14175 ) Change subject: Fix statestored /rpcz page .. Patch Set 2: Code-Review+2 carrying forward -- To view, visit http://gerrit.cloudera.org:8080/14175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf01c3af219e17728836c63b1e0ff10cbcb221ff Gerrit-Change-Number: 14175 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:51:39 + Gerrit-HasComments: No
[Impala-ASF-CR] Fix statestored /rpcz page
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14175 ) Change subject: Fix statestored /rpcz page .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4886/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf01c3af219e17728836c63b1e0ff10cbcb221ff Gerrit-Change-Number: 14175 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:50:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:44:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4885/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:44:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:40:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14157 ) Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4471/ : 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/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:35:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14157 ) Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:30:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/14173 ) Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/14173/1/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/14173/1/tests/webserver/test_web_pages.py@673 PS1, Line 673: regex = "(%s)|(%s)|(%s)" % (href_regex, form_regex, javascript_regex) > I missed this when reviewing the first patch, but it looks like the path is Done http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl File www/common-header.tmpl: http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl@76 PS1, Line 76: should > should Done http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl@76 PS1, Line 76: compatibility > nit: compatibility Done -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:26:44 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Hello Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14173 to look at the new patch set (#2). Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox integration .. IMPALA-8897 (part 2): Fix javascript to work with Knox integration This patch qualifies all urls accessed via javascript in the webui with the appropriate host:port in order to allow these urls to work when the connection to the webui is being proxied through Apache Knox. It accomplishes this with the function make_url(), which takes the href of a link from the page, which may have been rewritten by Knox, and appends a path to it. This patch also fixes on issue on the /admissions page, where resetting a pool's stats could fail due to the page being reloaded before the reset is executed. Fixed by moving the call to reload to the completion callback for the 'reset' endpoint. Testing: - Added a regex to test_knox_compatability that performs a rough check for places in the tmpl files where urls are used in javascript without calling make_url(). Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 --- M tests/webserver/test_web_pages.py M www/admission_controller.tmpl M www/common-header.tmpl M www/query_plan.tmpl M www/query_summary.tmpl M www/rpcz.tmpl 6 files changed, 38 insertions(+), 13 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/14173/2 -- To view, visit http://gerrit.cloudera.org:8080/14173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581 Gerrit-Change-Number: 14173 Gerrit-PatchSet: 2 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14157 ) Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4472/ : 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/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:24:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8825: Add additional counters to PlanRootSink
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14180 ) Change subject: IMPALA-8825: Add additional counters to PlanRootSink .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4470/ : 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/14180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9e101e2f3e2bf8324e149c780d35825ceecc036 Gerrit-Change-Number: 14180 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 18:24:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 19: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4469/ : 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/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 19 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 18:12:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8572: Log query events before Unregister() call.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14143 ) Change subject: IMPALA-8572: Log query events before Unregister() call. .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14143/4//COMMIT_MSG Commit Message: PS4: > 1) I've spent some time looking at it, especially reading git blame and old 1) the main reason to change the behaviour in this patch would be to simplify the code. I think it would generally be safer to log the audit events before query execution starts so that we default to the safe behaviour. 2) Yep, I see what you're saying - we use the completion of a wait thread as a signal so there's a dependency on the thread actually exiting. I think we could significantly simplify this if we change around the wait thread logic so that the wait thread runs LogQueryEvents() at the end, and use a condition variable instead of the BlockOnWait() logic. We could join on the thread in Done(). That eliminates a thread, and I think simplifying BlockOnWait() is a win too. -- To view, visit http://gerrit.cloudera.org:8080/14143 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I639b9c1acb9806b29292cd85be2863688453ca2e Gerrit-Change-Number: 14143 Gerrit-PatchSet: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: radford nguyen Gerrit-Comment-Date: Thu, 05 Sep 2019 17:59:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14157 to look at the new patch set (#7). Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. IMPALA-7312: Non-blocking mode for Fetch() RPC Adds the query option FETCH_ROWS_TIMEOUT_MS to control the client timeout when fetching rows. Set to 10 seconds by default to avoid unnecessary fetch requests. Timeout applies when result spooling is enabled or disabled. When result spooling is disabled, the timeout controls how long the client thread will wait for a single RowBatch to be produced by the coordinator fragment. When result spooling is enabled, a client can fetch multiple RowBatches at a time, so the timeout controls the total time spent waiting for RowBatches to be produced. Testing: * Added new tests to test_fetch.py * Running core tests Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d --- M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/buffered-plan-root-sink.cc M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h 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/hs2/test_fetch.py 9 files changed, 159 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14157/7 -- To view, visit http://gerrit.cloudera.org:8080/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14157 ) Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc File be/src/exec/blocking-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc@124 PS5, Line 124: bool timed_out = > Might be simpler to initialise to false. Done http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc@137 PS5, Line 137: (uint64_t) 1 > nit: shouldn't use c-style cast Done http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/buffered-plan-root-sink.cc@163 PS5, Line 163: PlanRootSink::fetch_rows_timeout_us() <= wait_timeout_timer.ElapsedTime(); > Might be simpler to initialise to false. I think we probably want to try at Done -- To view, visit http://gerrit.cloudera.org:8080/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 17:54:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Hello Michael Ho, Tim Armstrong, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14157 to look at the new patch set (#6). Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. IMPALA-7312: Non-blocking mode for Fetch() RPC Adds the query option FETCH_ROWS_TIMEOUT_MS to control the client timeout when fetching rows. Set to 10 seconds by default to avoid unnecessary fetch requests. Timeout applies when result spooling is enabled or disabled. When result spooling is disabled, the timeout controls how long the client thread will wait for a single RowBatch to be produced by the coordinator fragment. When result spooling is enabled, a client can fetch multiple RowBatches at a time, so the timeout controls the total time spent waiting for RowBatches to be produced. Testing: * Added new tests to test_fetch.py * Running core tests Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d --- M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/buffered-plan-root-sink.cc M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h 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/hs2/test_fetch.py 9 files changed, 159 insertions(+), 7 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/57/14157/6 -- To view, visit http://gerrit.cloudera.org:8080/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8825: Add additional counters to PlanRootSink
Sahil Takiar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14180 Change subject: IMPALA-8825: Add additional counters to PlanRootSink .. IMPALA-8825: Add additional counters to PlanRootSink Adds the counters RowsSent and RowsSentRate to the PLAN_ROOT_SINK section of the profile: PLAN_ROOT_SINK: - PeakMemoryUsage: 4.01 MB (4202496) - RowBatchGetWaitTime: 0.000ns - RowBatchSendWaitTime: 0.000ns - RowsSent: 10 (10) - RowsSentRate: 416.00 /sec RowsSent tracks the number of rows sent to the PlanRootSink via PlanRootSink::Send. RowsSentRate tracks the rate that rows are sent to the PlanRootSink. Adds the counters NumRowsFetched, RowMaterializationRate, and RowsMaterialized to the ImpalaServer section of the profile. ImpalaServer: - ClientFetchWaitTimer: 11.999ms - NumRowsFetched: 10 (10) - RowMaterializationRate: 9.00 /sec - RowMaterializationTimer: 1s007ms - RowsMaterialized: 10 (10) NumRowsFetched tracks the total number of rows fetched by the query, including rows fetched from the cache. RowMaterializationRate tracks the rate at which rows are materialized and RowsMaterialized tracks the number of rows materialized (RowMaterializationTimer already existed and tracks how much time is spent materializing rows). Testing: * Added tests to test_fetch_first.py and query_test/test_fetch.py * Enabled some tests in test_fetch_first.py that were pending the completion of IMPALA-8819 * Ran core tests Change-Id: Id9e101e2f3e2bf8324e149c780d35825ceecc036 --- M be/src/exec/blocking-plan-root-sink.cc M be/src/exec/buffered-plan-root-sink.cc M be/src/exec/plan-root-sink.cc M be/src/exec/plan-root-sink.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M tests/hs2/test_fetch_first.py A tests/query_test/test_fetch.py 8 files changed, 179 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/80/14180/1 -- To view, visit http://gerrit.cloudera.org:8080/14180 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id9e101e2f3e2bf8324e149c780d35825ceecc036 Gerrit-Change-Number: 14180 Gerrit-PatchSet: 1 Gerrit-Owner: Sahil Takiar
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 19: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4884/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 19 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 17:32:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13722
to look at the new patch set (#19).
Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..
IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
This enhancement introduces FORMAT clause for CAST() operator that is
applicable for casts between string types and timestamp types. Instead
of accepting SimpleDateFormat patterns the FORMAT clause supports
datetime patterns following the ISO:SQL:2016 standard.
Note, the CAST() operator without the FORMAT clause still uses
Impala's implementation of SimpleDateFormat handling. Similarly, the
existing conversion functions such as to_timestamp(), from_timestamp()
etc. remain unchanged and use SimpleDateFormat. Contrary to how these
functions work the FORMAT clause must specify a string literal and
cannot be used with any other kind of a string expression.
Milestone 1 contains all the format tokens covered by the SQL
standard. Further milestones will add more functionality on top of
this list to cover functionality provided by other RDBMS systems.
List of tokens implemented by this change:
- , YYY, YY, Y: Year tokens
- , RR: Round year tokens
- MM: Month (1-12)
- DD: Day (1-31)
- DDD: Day of year (1-366)
- HH, HH12: Hour of day (1-12)
- HH24: Hour of day (0-23)
- MI: Minute (0-59)
- SS: Second (0-59)
- S: Second of day (0-86399)
- FF, FF1, ..., FF9: Fractional second
- AM, PM, A.M., P.M.: Meridiem indicators
- TZH: Timezone hour (-99-+99)
- TZM: Timezone minute (0-99)
- Separators: - . / , ' ; : space
- ISO8601 date indicators (T, Z)
Some notes about the matching algorithm:
- The parsing algorithm uses these tokens in a case insensitive
manner.
- The separators are interchangeable with each other. For example a
'-' separator in the format will match with a '.' character in the
input.
- The length of the separator sequences is handled flexibly meaning
that a single separator character in the format for instance would
match with a multi-separator sequence in the input.
- In a string type to timestamp conversion the timezone offset tokens
are parsed, expected to match with the input but they don't adjust
the result as the input is already expected to be in UTC format.
Usage example:
SELECT CAST('01-02-2019' AS TIMESTAMP FORMAT 'MM-DD-');
SELECT CAST('2019.10.10 13:30:40.123456 +01:30' AS TIMESTAMP
FORMAT '-MM-DD HH24:MI:SS.FF9 TZH:TZM');
SELECT CAST(timestamp_column as STRING
FORMAT " MM HH12 YY") from some_table;
Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
---
M be/src/benchmarks/convert-timestamp-benchmark.cc
M be/src/benchmarks/parse-timestamp-benchmark.cc
M be/src/common/init.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/CMakeLists.txt
A be/src/exprs/cast-format-expr.cc
A be/src/exprs/cast-format-expr.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/date-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.h
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/date-parse-util.cc
M be/src/runtime/date-parse-util.h
M be/src/runtime/date-test.cc
M be/src/runtime/date-value.cc
M be/src/runtime/date-value.h
A be/src/runtime/datetime-iso-sql-format-parser.cc
A be/src/runtime/datetime-iso-sql-format-parser.h
A be/src/runtime/datetime-iso-sql-format-tokenizer.cc
A be/src/runtime/datetime-iso-sql-format-tokenizer.h
D be/src/runtime/datetime-parse-util.h
A be/src/runtime/datetime-parser-common.cc
A be/src/runtime/datetime-parser-common.h
R be/src/runtime/datetime-simple-date-format-parser.cc
A be/src/runtime/datetime-simple-date-format-parser.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/testutil/random-vector-generators.h
M be/src/util/dict-test.cc
M be/src/util/min-max-filter-test.cc
M be/src/util/string-parser.h
M common/thrift/Exprs.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A
testdata/workloads/functional-query/queries/QueryTest/cast_format_from_table.test
M testdata/workloads/functional-query/queries/QueryTest/date.test
A tests/query_test/test_cast_with_format.py
54 files changed, 3,461 insertions(+), 865 deletions(-)
git
[Impala-ASF-CR] IMPALA-8897 (part 2): Fix javascript to work with Knox integration
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/14173 )
Change subject: IMPALA-8897 (part 2): Fix javascript to work with Knox
integration
..
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14173/1/tests/webserver/test_web_pages.py
File tests/webserver/test_web_pages.py:
http://gerrit.cloudera.org:8080/#/c/14173/1/tests/webserver/test_web_pages.py@673
PS1, Line 673: results = grep_dir("/home/thomas/Impala/www", regex,
".*\.tmpl")
I missed this when reviewing the first patch, but it looks like the path is
wrong here.
http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl
File www/common-header.tmpl:
http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl@76
PS1, Line 76: compatability
nit: compatibility
http://gerrit.cloudera.org:8080/#/c/14173/1/www/common-header.tmpl@76
PS1, Line 76: shuold
should
--
To view, visit http://gerrit.cloudera.org:8080/14173
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3de9fd1bbb8bb38ce63b3160fcafd33eb0530581
Gerrit-Change-Number: 14173
Gerrit-PatchSet: 1
Gerrit-Owner: Thomas Tauber-Marshall
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Thu, 05 Sep 2019 17:11:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] Fix statestored /rpcz page
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14175 ) Change subject: Fix statestored /rpcz page .. Patch Set 1: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/14175/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14175/1//COMMIT_MSG@7 PS1, Line 7: Fix statestored /rpcz page Might be worth filing a JIRA for this for tracking. OK to ignore. -- To view, visit http://gerrit.cloudera.org:8080/14175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaf01c3af219e17728836c63b1e0ff10cbcb221ff Gerrit-Change-Number: 14175 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Tauber-Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 17:06:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 18: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4468/ : 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/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 18 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 16:17:46 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7312: Non-blocking mode for Fetch() RPC
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/14157 ) Change subject: IMPALA-7312: Non-blocking mode for Fetch() RPC .. Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc File be/src/exec/blocking-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc@124 PS5, Line 124: bool timed_out = Might be simpler to initialise to false. http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/blocking-plan-root-sink.cc@137 PS5, Line 137: (uint64_t) 1 nit: shouldn't use c-style cast http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/buffered-plan-root-sink.cc File be/src/exec/buffered-plan-root-sink.cc: http://gerrit.cloudera.org:8080/#/c/14157/5/be/src/exec/buffered-plan-root-sink.cc@163 PS5, Line 163: PlanRootSink::fetch_rows_timeout_us() <= wait_timeout_timer.ElapsedTime(); Might be simpler to initialise to false. I think we probably want to try at least once to get rows otherwise things could (potentially) end up livelocked in some extreme cases. -- To view, visit http://gerrit.cloudera.org:8080/14157 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I331acaba23a65dab43cca48e9dc0dc957b9c632d Gerrit-Change-Number: 14157 Gerrit-PatchSet: 5 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Thu, 05 Sep 2019 16:17:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 17: (7 comments) http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h File be/src/exprs/cast-format-expr.h: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@35 PS17, Line 35: virtual > nit: 'virtual' keyword is not necessary. Done http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@37 PS17, Line 37: virtual > nit: same Done http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@46 PS17, Line 46: FunctionContext > const Done http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.cc File be/src/exprs/cast-format-expr.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.cc@64 PS17, Line 64: FunctionContext > const FunctionContext* Done http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc@181 PS17, Line 181: int buf_len = format_ctx->fmt_out_len + 1; > DCHECK(buf_len <= IsoSqlFormatTokenizer::MAX_FORMAT_LENGTH); There are tokens where the input they accept is actually longer than the length of the token. e.g. MONTH, DAY, DY, FF[4-9] AM, PM. So limiting the input string length not to be longer than the max length of the format string won't be correct. http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc@203 PS17, Line 203: int buf_len = format_ctx->fmt_out_len + 1; > DCHECK(buf_len <= IsoSqlFormatTokenizer::MAX_FORMAT_LENGTH); Same as above. http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/runtime/datetime-iso-sql-format-tokenizer.cc File be/src/runtime/datetime-iso-sql-format-tokenizer.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@110 PS17, Line 110: DCHECK(*current_pos <= str_end); > Please revisit this DCHECK and the one in L113. On the callsite I prevent calling this function when current_pos == str_end so I modified the DCHECK in this line. -- To view, visit http://gerrit.cloudera.org:8080/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 17 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 15:37:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Hello Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13722
to look at the new patch set (#18).
Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
..
IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
This enhancement introduces FORMAT clause for CAST() operator that is
applicable for casts between string types and timestamp types. Instead
of accepting SimpleDateFormat patterns the FORMAT clause supports
datetime patterns following the ISO:SQL:2016 standard.
Note, the CAST() operator without the FORMAT clause still uses
Impala's implementation of SimpleDateFormat handling. Similarly, the
existing conversion functions such as to_timestamp(), from_timestamp()
etc. remain unchanged and use SimpleDateFormat. Contrary to how these
functions work the FORMAT clause must specify a string literal and
cannot be used with any other kind of a string expression.
Milestone 1 contains all the format tokens covered by the SQL
standard. Further milestones will add more functionality on top of
this list to cover functionality provided by other RDBMS systems.
List of tokens implemented by this change:
- , YYY, YY, Y: Year tokens
- , RR: Round year tokens
- MM: Month (1-12)
- DD: Day (1-31)
- DDD: Day of year (1-366)
- HH, HH12: Hour of day (1-12)
- HH24: Hour of day (0-23)
- MI: Minute (0-59)
- SS: Second (0-59)
- S: Second of day (0-86399)
- FF, FF1, ..., FF9: Fractional second
- AM, PM, A.M., P.M.: Meridiem indicators
- TZH: Timezone hour (-99-+99)
- TZM: Timezone minute (0-99)
- Separators: - . / , ' ; : space
- ISO8601 date indicators (T, Z)
Some notes about the matching algorithm:
- The parsing algorithm uses these tokens in a case insensitive
manner.
- The separators are interchangeable with each other. For example a
'-' separator in the format will match with a '.' character in the
input.
- The length of the separator sequences is handled flexibly meaning
that a single separator character in the format for instance would
match with a multi-separator sequence in the input.
- In a string type to timestamp conversion the timezone offset tokens
are parsed, expected to match with the input but they don't adjust
the result as the input is already expected to be in UTC format.
Usage example:
SELECT CAST('01-02-2019' AS TIMESTAMP FORMAT 'MM-DD-');
SELECT CAST('2019.10.10 13:30:40.123456 +01:30' AS TIMESTAMP
FORMAT '-MM-DD HH24:MI:SS.FF9 TZH:TZM');
SELECT CAST(timestamp_column as STRING
FORMAT " MM HH12 YY") from some_table;
Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23
---
M be/src/benchmarks/convert-timestamp-benchmark.cc
M be/src/benchmarks/parse-timestamp-benchmark.cc
M be/src/common/init.cc
M be/src/exec/text-converter.inline.h
M be/src/exprs/CMakeLists.txt
A be/src/exprs/cast-format-expr.cc
A be/src/exprs/cast-format-expr.h
M be/src/exprs/cast-functions-ir.cc
M be/src/exprs/date-functions-ir.cc
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr-evaluator.h
M be/src/exprs/scalar-expr.cc
M be/src/exprs/scalar-expr.h
M be/src/exprs/timestamp-functions-ir.cc
M be/src/exprs/timestamp-functions.cc
M be/src/exprs/timestamp-functions.h
M be/src/runtime/CMakeLists.txt
M be/src/runtime/date-parse-util.cc
M be/src/runtime/date-parse-util.h
M be/src/runtime/date-test.cc
M be/src/runtime/date-value.cc
M be/src/runtime/date-value.h
A be/src/runtime/datetime-iso-sql-format-parser.cc
A be/src/runtime/datetime-iso-sql-format-parser.h
A be/src/runtime/datetime-iso-sql-format-tokenizer.cc
A be/src/runtime/datetime-iso-sql-format-tokenizer.h
D be/src/runtime/datetime-parse-util.h
A be/src/runtime/datetime-parser-common.cc
A be/src/runtime/datetime-parser-common.h
R be/src/runtime/datetime-simple-date-format-parser.cc
A be/src/runtime/datetime-simple-date-format-parser.h
M be/src/runtime/runtime-state.cc
M be/src/runtime/timestamp-parse-util.cc
M be/src/runtime/timestamp-parse-util.h
M be/src/runtime/timestamp-test.cc
M be/src/runtime/timestamp-value.cc
M be/src/runtime/timestamp-value.h
M be/src/service/impala-server.cc
M be/src/service/query-options.cc
M be/src/service/query-options.h
M be/src/testutil/random-vector-generators.h
M be/src/util/dict-test.cc
M be/src/util/min-max-filter-test.cc
M be/src/util/string-parser.h
M common/thrift/Exprs.thrift
M common/thrift/ImpalaInternalService.thrift
M common/thrift/ImpalaService.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
A
testdata/workloads/functional-query/queries/QueryTest/cast_format_from_table.test
M testdata/workloads/functional-query/queries/QueryTest/date.test
A tests/query_test/test_cast_with_format.py
54 files changed, 3,461 insertions(+), 865 deletions(-)
git
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 ) Change subject: IMPALA-8755: Frontend support for Z-ordering .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4467/ : 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/13955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d Gerrit-Change-Number: 13955 Gerrit-PatchSet: 12 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 05 Sep 2019 11:10:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. IMPALA-8797: Support database and table blacklist Add catalogd and coordinator startup options for database and table blacklist. Blacklisted dbs/tables will be skipped in loading. Users won't see them when getting database/table list, e.g. in SHOW DATABASES/TABLES. Dropping/creating blacklisted databases/tables/views are not allowed too. Implementation: Catalogd and coordinators parses the --blacklisted_dbs and --blacklisted_tables options in startup. Blacklist checks are added in catalogd when loading the metadata and in coordinators when analysing DDL requests for create/drop. Catalogd will also check blacklist when executing DDL requests from coordinators in case their blacklists are inconsistent. Motivation: By default, it's used to blacklist "sys" and "information_schema" databases from Hive. Admin can use them to specify any databases/tables that are not suitable for Impala to query. Tests: - Add java unit tests for blacklist related functions - Add a custom cluster test: test_blacklisted_dbs_and_tables.py - Ran CORE tests Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Reviewed-on: http://gerrit.cloudera.org:8080/14134 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/analysis/AlterTableOrViewRenameStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateDbStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateViewStmt.java M fe/src/main/java/org/apache/impala/analysis/TableName.java M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A fe/src/main/java/org/apache/impala/util/CatalogBlacklistUtils.java A fe/src/test/java/org/apache/impala/util/CatalogBlacklistUtilsTest.java A tests/custom_cluster/test_blacklisted_dbs_and_tables.py 14 files changed, 542 insertions(+), 19 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 14 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. Patch Set 13: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 13 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 05 Sep 2019 11:10:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13955 ) Change subject: IMPALA-8755: Frontend support for Z-ordering .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java: http://gerrit.cloudera.org:8080/#/c/13955/12/fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java@1977 PS12, Line 1977: "SORT BY ZORDER does not support column types: STRING, VARCHAR(*), FLOAT, " + line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/13955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d Gerrit-Change-Number: 13955 Gerrit-PatchSet: 12 Gerrit-Owner: Norbert Luksa Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Norbert Luksa Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 05 Sep 2019 10:30:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Norbert Luksa has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/13955 ) Change subject: IMPALA-8755: Frontend support for Z-ordering .. IMPALA-8755: Frontend support for Z-ordering Extended the SQL grammar with an optional and a default flag for SORT BY, namely ZORDER and LEXICAL. If set, the new 'sort.algorithm' table property will be set to ZORDER and the information will sink down to the backend. The default order is indicated by LEXICAL and can be omitted. Examples are: CREATE TABLE t (a INT, b INT) PARTITIONED BY (c INT) SORT BY ZORDER (a, b); CREATE TABLE t SORT BY ZORDER (int_col,id) LIKE u; CREATE TABLE t LIKE PARQUET '/foo' SORT BY ZORDER (id,zip); ALTER TABLE t SORT BY ZORDER (int_col,id); The following two are the same statements: CREATE TABLE t (a INT, b INT) SORT BY (a, b); CREATE TABLE t (a INT, b INT) SORT BY LEXICAL (a, b); For strings, varchars, floats and doubles Z-ordering is currently not supported. It's not suitable for strings and varchars, but support can be added for floats and doubles later. The supported types are: boolean, int types, decimals, date, timestamp, and char. Currently ZORDER has the same functionality as a simple SORT BY clause, therefore hidden behind a feature flag: unlock_zorder. The custom sorting with Z-ordering will be in a different commit later. Testing: * Added tests for the ZORDER option for every SORT BY test. * Modified some tests by adding the LEXICAL option. * The .test workloads are temporarily put in separate test files in order to set up the feature flag. These tests are run from tests/custom_cluster/test_zorder.py which is a duplication of the relevant tests, but with CustomClusterTestSuite decorator. Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M common/thrift/DataSinks.thrift M common/thrift/JniCatalog.thrift M common/thrift/PlanNodes.thrift M common/thrift/Types.thrift M fe/src/main/cup/sql-parser.cup M fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java M fe/src/main/java/org/apache/impala/analysis/AlterTableSortByStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeFileStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableLikeStmt.java M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java M fe/src/main/java/org/apache/impala/analysis/DeleteStmt.java M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java M fe/src/main/java/org/apache/impala/analysis/SortInfo.java M fe/src/main/java/org/apache/impala/analysis/TableDef.java M fe/src/main/java/org/apache/impala/analysis/ToSqlUtils.java M fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java M fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java M fe/src/main/java/org/apache/impala/planner/ExchangeNode.java M fe/src/main/java/org/apache/impala/planner/HdfsTableSink.java M fe/src/main/java/org/apache/impala/planner/PlanNode.java M fe/src/main/java/org/apache/impala/planner/Planner.java M fe/src/main/java/org/apache/impala/planner/SortNode.java M fe/src/main/java/org/apache/impala/planner/TableSink.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/jflex/sql-scanner.flex M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java M fe/src/test/java/org/apache/impala/analysis/AnalyzeKuduDDLTest.java M fe/src/test/java/org/apache/impala/analysis/ParserTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java M fe/src/test/java/org/apache/impala/analysis/ToSqlUtilsTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/planner/PlannerTest.java A testdata/workloads/functional-planner/queries/PlannerTest/insert-sort-by-zorder.test A testdata/workloads/functional-query/queries/QueryTest/alter-table-zorder.test A testdata/workloads/functional-query/queries/QueryTest/create-table-as-select-zorder.test A testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-zorder.test A testdata/workloads/functional-query/queries/QueryTest/create-table-like-table-zorder.test A testdata/workloads/functional-query/queries/QueryTest/create-table-zorder.test A testdata/workloads/functional-query/queries/QueryTest/show-create-table-zorder.test M testdata/workloads/functional-query/queries/QueryTest/show-create-table.test A tests/custom_cluster/test_zorder.py 45 files changed, 1,796 insertions(+), 166 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/55/13955/12 -- To view, visit http://gerrit.cloudera.org:8080/13955 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset
[Impala-ASF-CR] IMPALA-8755: Frontend support for Z-ordering
Norbert Luksa has posted comments on this change. (
http://gerrit.cloudera.org:8080/13955 )
Change subject: IMPALA-8755: Frontend support for Z-ordering
..
Patch Set 11:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13955/9//COMMIT_MSG@30
PS9, Line 30:
> hmm, indeed, unlock_zorder_sort would be slightly better. (nit)
Done
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java
File fe/src/main/java/org/apache/impala/analysis/TableDef.java:
http://gerrit.cloudera.org:8080/#/c/13955/9/fe/src/main/java/org/apache/impala/analysis/TableDef.java@397
PS9, Line 397: throw new AnalysisException(String.format("SORT BY ZORDER does
not support "
: + "column type: %s", colType.toSql()));
> Hmm, It might be more usable if you printed out all the unsupported columns
Sounds good, will change it that way.
--
To view, visit http://gerrit.cloudera.org:8080/13955
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie122002ca8f52ca2c1e1ec8ff1d476ae1f4f875d
Gerrit-Change-Number: 13955
Gerrit-PatchSet: 11
Gerrit-Owner: Norbert Luksa
Gerrit-Reviewer: Gabor Kaszab
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Norbert Luksa
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 05 Sep 2019 10:29:47 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/13722 ) Change subject: IMPALA-8703: ISO:SQL:2016 datetime patterns - Milestone 1 .. Patch Set 17: (7 comments) Couple more comments. I can +2 it once these are done. http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h File be/src/exprs/cast-format-expr.h: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@35 PS17, Line 35: virtual nit: 'virtual' keyword is not necessary. 'override' keyword already indicates that this is an override of a virtual function in the bases class. http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@37 PS17, Line 37: virtual nit: same http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.h@46 PS17, Line 46: FunctionContext const http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.cc File be/src/exprs/cast-format-expr.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-format-expr.cc@64 PS17, Line 64: FunctionContext const FunctionContext* http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc File be/src/exprs/cast-functions-ir.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc@181 PS17, Line 181: int buf_len = format_ctx->fmt_out_len + 1; DCHECK(buf_len <= IsoSqlFormatTokenizer::MAX_FORMAT_LENGTH); http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/exprs/cast-functions-ir.cc@203 PS17, Line 203: int buf_len = format_ctx->fmt_out_len + 1; DCHECK(buf_len <= IsoSqlFormatTokenizer::MAX_FORMAT_LENGTH); http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/runtime/datetime-iso-sql-format-tokenizer.cc File be/src/runtime/datetime-iso-sql-format-tokenizer.cc: http://gerrit.cloudera.org:8080/#/c/13722/17/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@110 PS17, Line 110: DCHECK(*current_pos <= str_end); Please revisit this DCHECK and the one in L113. If you use '<=' here, you should use '>=' in L113. -- To view, visit http://gerrit.cloudera.org:8080/13722 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I19d8d097a45ae6f103b6cd1b2d81aad38dfd9e23 Gerrit-Change-Number: 13722 Gerrit-PatchSet: 17 Gerrit-Owner: Gabor Kaszab Gerrit-Reviewer: Attila Jeges Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 05 Sep 2019 08:37:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4882/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 13 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 05 Sep 2019 07:03:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 13 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 05 Sep 2019 07:03:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8797: Support database and table blacklist
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14134 ) Change subject: IMPALA-8797: Support database and table blacklist .. Patch Set 12: Code-Review+2 Carry on +2 of Bharath -- To view, visit http://gerrit.cloudera.org:8080/14134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I02dbb07f8e08793b57b2a88d09b30fd32cff26dc Gerrit-Change-Number: 14134 Gerrit-PatchSet: 12 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 05 Sep 2019 07:02:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8803: Coordinator should release admitted memory per-backend
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/14104 ) Change subject: IMPALA-8803: Coordinator should release admitted memory per-backend .. Patch Set 7: (15 comments) http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc File be/src/runtime/coordinator-backend-resource-state.cc: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@81 PS7, Line 81: num_pending_releasable_or_released_ Would it be simpler to track the negation (i.e. num_in_use_) instead ? We only need to decrement num_in_use_ here vs bumping this counter at multiple places. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@95 PS7, Line 95: is_coordinator_the_last_unreleased_backend If I understand it correctly, there is no adverse effect for this heuristics even in the case where there are no coordinator fragments, right? http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@106 PS7, Line 106: } DCHECK_GE(num_pending_, 0); http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-resource-state.cc@117 PS7, Line 117: DCHECK( DCHECK_NE http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h File be/src/runtime/coordinator-backend-state.h: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@457 PS7, Line 457: PENDING state or RELESABLE ? http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@460 PS7, Line 460: /// released. A no-op if the BackendResourceState is closed already. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@465 PS7, Line 465: /// BackendStates have been released. This may be called even after CloseAndGetUnreleasedBackends(). http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/runtime/coordinator-backend-state.h@509 PS7, Line 509: BackendResourceState is closed, May help to clarify what may and may not happen after transitioning to the "closed_" state. http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h@851 PS7, Line 851: specificied typo http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.h@852 PS7, Line 852: UpdateStatsForHost( nit: This name is a bit too similar to UpdateHostStats() and they seem to serve similar functions too. I wonder if it's more consistent to just overload UpdateHostStats() or rename it to UpdateHostStatsInternal() ? http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@406 PS7, Line 406: DCHECK(false) << strings::Substitute( : "Error: Cannot find host address $0 for query $1.", PrintThrift(host_addr), : PrintId(schedule.query_id())); Shouldn't this be LOG(ERROR) or something in addition to DCHECK(false); ? http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@408 PS7, Line 408: PrintId(schedule.query_id())); Should we add a continue; in this case ? http://gerrit.cloudera.org:8080/#/c/14104/7/be/src/scheduling/admission-controller.cc@1531 PS7, Line 1531: } DCHECK_EQ(num_release_backends_.find(schedule->query_id()), num_release_backends_.end()); http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_executor_groups.py File tests/custom_cluster/test_executor_groups.py: http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_executor_groups.py@236 PS7, Line 236: 3 Why this change ? http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_result_spooling.py File tests/custom_cluster/test_result_spooling.py: http://gerrit.cloudera.org:8080/#/c/14104/7/tests/custom_cluster/test_result_spooling.py@62 PS7, Line 62: self.wait_for_state(handle, self.client.QUERY_STATES['FINISHED'], timeout) Is there a potential race here ? If I understand it correctly, we may reach the 'FINISHED' state after Open() completes in the exec node. For the merging exchange, it may only fetch the first row batch from each sender. So there seems to be a chance which not all results have been spooled yet given the limit 2000. -- To view, visit http://gerrit.cloudera.org:8080/14104 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I88bb11e0ede7574568020e0277dd8ac8d2586dc9 Gerrit-Change-Number: 14104 Gerrit-PatchSet: 7 Gerrit-Owner: Sahil Takiar
