[Impala-ASF-CR] IMPALA-10076: Reduce partition level update logs
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16375 ) Change subject: IMPALA-10076: Reduce partition level update logs .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6431/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/16375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48946b2f8b0be1e73988092d03a004836f1b368 Gerrit-Change-Number: 16375 Gerrit-PatchSet: 4 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Wed, 16 Sep 2020 06:22:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 6: Code-Review+2
(1 comment)
LGTM. Just have a minor comment on the test. Feel free to carry on my +2 when
it's resolved.
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py
File tests/custom_cluster/test_shell_interactive.py:
http://gerrit.cloudera.org:8080/#/c/16406/6/tests/custom_cluster/test_shell_interactive.py@87
PS6, Line 87: proc.expect("Query State: FINISHED")
Can we verify the absence of the original profile? I.e. something like this:
import pexpect
try:
proc.expect("Failed Query Runtime Profile", timeout=3)
assert False
except pexpect.TIMEOUT:
assert True
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 6
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 16 Sep 2020 03:20:00 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7183/ : 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/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 02:09:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: reviewers* -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 01:50:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16406 ) Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. Patch Set 2: > Do you plan to let anyone else to take a look? If not, I can bump to +2 after > tests are added. No plans to add other reviews. -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 2 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar Gerrit-Comment-Date: Wed, 16 Sep 2020 01:49:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Sahil Takiar has posted comments on this change. (
http://gerrit.cloudera.org:8080/16406 )
Change subject: IMPALA-9229: impala-shell 'profile' to show original and
retried queries
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1042
PS5, Line 1042: failed_thrift_profiles.emplace_back(*thrift_profi
> nit: this is already done in __set_failed_thrift_profiles()
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1052
PS5, Line 1052: original_profiles.emplace_back(ss->str());
> nit: this is already done in __set_failed_profiles()
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1056
PS5, Line 1056:
> nit: only the 'format' of the request is used. We can refactor it to a TRun
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1121
PS5, Line 1121: GetRuntimeProfileOu
> nit: 4 spaces indention
Not sure why, but ClangFormat makes it like this.
http://gerrit.cloudera.org:8080/#/c/16406/5/be/src/service/impala-hs2-server.cc@1132
PS5, Line 1132: if (was_retried && request.include_query_attempts) {
: if (request.format == TRuntimeProfileFormat::THRIFT) {
: SetFailedProfile(&thrift_profile, return_val);
: } else if (request.format == TRuntimeProfileFormat::JSON) {
: JSONProfileToStringProfile(&ss, &json_profile);
: SetFailedProfile(&ss, return_val);
: } else {
: DCHECK(request.format == TRuntimeProfileFormat::STRING
: || request.format == TRuntimeProfileFormat::BASE64);
:
> nit: the 3 if branches have the same conditions as SetProfile() at line 105
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py
File shell/impala_shell.py:
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@110
PS5, Line 110: ons
> typo then?
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@111
PS5, Line 111: thi
> typo then?
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1004
PS5, Line 1004: db_
> nit: 2 spaces indention
Done
http://gerrit.cloudera.org:8080/#/c/16406/5/shell/impala_shell.py@1071
PS5, Line 1071: prettytable =
self.construct_table_with_header(column_names)
: formatter = PrettyOutputFormatter(pret
> nit: may be helpful to print the three valid values as well.
Done
--
To view, visit http://gerrit.cloudera.org:8080/16406
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65
Gerrit-Change-Number: 16406
Gerrit-PatchSet: 2
Gerrit-Owner: Sahil Takiar
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Sahil Takiar
Gerrit-Comment-Date: Wed, 16 Sep 2020 01:49:25 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9229: impala-shell 'profile' to show original and retried queries
Hello Quanlong Huang, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/16406 to look at the new patch set (#6). Change subject: IMPALA-9229: impala-shell 'profile' to show original and retried queries .. IMPALA-9229: impala-shell 'profile' to show original and retried queries Currently, the impala-shell 'profile' command only returns the profile for the most recent profile attempt. There is no way to get the original query profile (the profile of the first query attempt that failed) from the impala-shell. This patch modifies TGetRuntimeProfileReq and TGetRuntimeProfileResp to add support for returning both the original and retried profiles for a retried query. When a query is retried, TGetRuntimeProfileResp currently contains the profile for the most recent query attempt. TGetRuntimeProfileReq has a new field called 'include_query_attempts' and when it is set to true, the TGetRuntimeProfileResp will include all failed profiles in a new field called failed_profiles / failed_thrift_profiles. impala-shell has been modified so the 'profile' command has a new set of options. The syntax is now: PROFILE [ALL | LATEST | ORIGINAL] If 'ALL' is specified, both the latest and original profiles are printed. If 'LATEST' is specified, only the latest profile is printed. If 'ORIGINAL' is printed, only the original profile is printed. The default behavior is equivalent to specifying 'LATEST' (which is the current behavior before this patch as well). Support for this has only been added to HS2 given that Beeswax is being deprecated soon. The new 'profile' options have no affect when the Beeswax protocol is used. Most of the code change is in impala-hs2-server and impala-server; a lot of the GetRuntimeProfile code has been re-factored. Testing: * Added new impala-shell tests * Ran core tests Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 --- M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-http-handler.cc M be/src/service/impala-server.cc M be/src/service/impala-server.h M common/thrift/ImpalaService.thrift M shell/impala_client.py M shell/impala_shell.py M tests/custom_cluster/test_shell_interactive.py M tests/shell/test_shell_commandline.py M tests/shell/util.py 13 files changed, 498 insertions(+), 125 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/16406/6 -- To view, visit http://gerrit.cloudera.org:8080/16406 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I89cee02947b311e7bf9c7274f47dfc7214c1bb65 Gerrit-Change-Number: 16406 Gerrit-PatchSet: 6 Gerrit-Owner: Sahil Takiar Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Sahil Takiar
[Impala-ASF-CR] IMPALA-9227: Add coverage for query retries caused by RPC failure
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16456 ) Change subject: IMPALA-9227: Add coverage for query retries caused by RPC failure .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7182/ : 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/16456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6c800a764b22c72d5c408e14fd1bdea5336d7d82 Gerrit-Change-Number: 16456 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Comment-Date: Tue, 15 Sep 2020 22:13:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9227: Add coverage for query retries caused by RPC failure
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16456 Change subject: IMPALA-9227: Add coverage for query retries caused by RPC failure .. IMPALA-9227: Add coverage for query retries caused by RPC failure Added two new unit tests for query retries triggered by RPC failures. The RPC failures are simulated by using debug_action to inject RPC fault. Testing: - Passed test_query_retries.py, including the new test cases. Change-Id: I6c800a764b22c72d5c408e14fd1bdea5336d7d82 --- M tests/custom_cluster/test_query_retries.py 1 file changed, 104 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/56/16456/1 -- To view, visit http://gerrit.cloudera.org:8080/16456 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6c800a764b22c72d5c408e14fd1bdea5336d7d82 Gerrit-Change-Number: 16456 Gerrit-PatchSet: 1 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall
[Impala-ASF-CR] IMPALA-9636: Don't run retried query on the blacklisted nodes
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16369 ) Change subject: IMPALA-9636: Don't run retried query on the blacklisted nodes .. Patch Set 19: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/16369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Gerrit-Change-Number: 16369 Gerrit-PatchSet: 19 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 15 Sep 2020 20:31:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9636: Don't run retried query on the blacklisted nodes
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16369 ) Change subject: IMPALA-9636: Don't run retried query on the blacklisted nodes .. IMPALA-9636: Don't run retried query on the blacklisted nodes When a node is blacklisted, it is only placed on the blacklist for a certain period of time. For the current implementation, it is possible that the retried query could end up running on the node that it blacklisted during its original attempt. To avoid same failure for the retried query, we should not schedule query fragment instances on the blacklisted nodes which caused the original query to fail. This patch filters out the executors from executor group for those nodes which are blacklisted during its original attempt when make schedule for the retried query. Adds new test cases test_retry_exec_rpc_failure_before_admin_delay() and test_retry_query_failure_all_executors_blacklisted() for retried queries which are triggered by RPC failure and blacklist timeout are triggered by adding delay before admission. Testing: - Passed test_query_retries.py, including the new test cases. - Passed core tests. Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Reviewed-on: http://gerrit.cloudera.org:8080/16369 Reviewed-by: Sahil Takiar Tested-by: Impala Public Jenkins --- M be/src/runtime/coordinator.cc M be/src/runtime/coordinator.h M be/src/runtime/query-driver.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M be/src/scheduling/executor-group.cc M be/src/scheduling/executor-group.h M be/src/service/client-request-state.cc M be/src/service/client-request-state.h M tests/custom_cluster/test_query_retries.py 10 files changed, 241 insertions(+), 5 deletions(-) Approvals: Sahil Takiar: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/16369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Gerrit-Change-Number: 16369 Gerrit-PatchSet: 20 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou
[Impala-ASF-CR] IMPALA-10051: impala-shell exits with ValueError with WITH clauses
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/16389 ) Change subject: IMPALA-10051: impala-shell exits with ValueError with WITH clauses .. Patch Set 5: > Patch Set 5: > > Hi Fredy, > Just checking in if you had some time to review the current version. LGTM. Thanks for fixing this! -- To view, visit http://gerrit.cloudera.org:8080/16389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I442d3bc65b90a55c73c847948d5179a8586d71ad Gerrit-Change-Number: 16389 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 15 Sep 2020 16:30:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10051: impala-shell exits with ValueError with WITH clauses
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/16389 ) Change subject: IMPALA-10051: impala-shell exits with ValueError with WITH clauses .. Patch Set 5: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/16389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I442d3bc65b90a55c73c847948d5179a8586d71ad Gerrit-Change-Number: 16389 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 15 Sep 2020 16:30:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9636: Don't run retried query on the blacklisted nodes
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16369 ) Change subject: IMPALA-9636: Don't run retried query on the blacklisted nodes .. Patch Set 19: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6430/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/16369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Gerrit-Change-Number: 16369 Gerrit-PatchSet: 19 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 15 Sep 2020 15:27:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9636: Don't run retried query on the blacklisted nodes
Sahil Takiar has posted comments on this change. ( http://gerrit.cloudera.org:8080/16369 ) Change subject: IMPALA-9636: Don't run retried query on the blacklisted nodes .. Patch Set 19: Code-Review+2 LGTM -- To view, visit http://gerrit.cloudera.org:8080/16369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00bc1b5026efbd0670ffbe57bcebc457d34cb105 Gerrit-Change-Number: 16369 Gerrit-PatchSet: 19 Gerrit-Owner: Wenzhe Zhou Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Wenzhe Zhou Gerrit-Comment-Date: Tue, 15 Sep 2020 15:27:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10051: impala-shell exits with ValueError with WITH clauses
Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/16389 ) Change subject: IMPALA-10051: impala-shell exits with ValueError with WITH clauses .. Patch Set 5: Hi Fredy, Just checking in if you had some time to review the current version. -- To view, visit http://gerrit.cloudera.org:8080/16389 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I442d3bc65b90a55c73c847948d5179a8586d71ad Gerrit-Change-Number: 16389 Gerrit-PatchSet: 5 Gerrit-Owner: Tamas Mate Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Tamas Mate Gerrit-Comment-Date: Tue, 15 Sep 2020 14:53:59 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] MPALA-10075: Reuse unchanged partition instances
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16392 ) Change subject: [WIP] MPALA-10075: Reuse unchanged partition instances .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/16392/1/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java: http://gerrit.cloudera.org:8080/#/c/16392/1/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1548 PS1, Line 1548: hasIncrementalStats_ == oldInstan > It's included at line 1540. Done -- To view, visit http://gerrit.cloudera.org:8080/16392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2dd645c260d271291021e52fdac4b74924df1170 Gerrit-Change-Number: 16392 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Tue, 15 Sep 2020 14:25:40 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Generate test-classpath.txt
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16450 ) Change subject: Generate test-classpath.txt .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7181/ : 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/16450 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I47e4340fde9a5678d94459f676d8894dc5da14dc Gerrit-Change-Number: 16450 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Comment-Date: Tue, 15 Sep 2020 14:06:23 + Gerrit-HasComments: No
[Impala-ASF-CR] Generate test-classpath.txt
Zoltan Borok-Nagy has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16450 Change subject: Generate test-classpath.txt .. Generate test-classpath.txt Currently the hive-jdbc jar is added under the scope 'test'. Therefore it won't be included in the build-classpath.txt because it only includes jars from the 'runtime' scope. Because of this run-hive-jdbc.sh throws ClassNotFoundException exception. This commit adds a new execution 'write-test-classpath' to pom.xml. It creates a test-classpath.txt. This can be used by scripts that need classes from the 'test' scope. Modified set-classpath.sh as well. If its first argument is 'test', then it also adds the contents of test-classpath.txt to the CLASSPATH. Now run-jdbc-client.sh sources 'set-classpath.sh test'. Change-Id: I47e4340fde9a5678d94459f676d8894dc5da14dc --- M bin/run-jdbc-client.sh M bin/set-classpath.sh M fe/pom.xml 3 files changed, 18 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/50/16450/1 -- To view, visit http://gerrit.cloudera.org:8080/16450 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I47e4340fde9a5678d94459f676d8894dc5da14dc Gerrit-Change-Number: 16450 Gerrit-PatchSet: 1 Gerrit-Owner: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-10164: Supporting HadoopCatalog for Iceberg table
wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16446 ) Change subject: IMPALA-10164: Supporting HadoopCatalog for Iceberg table .. Patch Set 2: (8 comments) Hi Gabor, thanks for your review, I've already modify code. http://gerrit.cloudera.org:8080/#/c/16446/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16446/1//COMMIT_MSG@20 PS1, Line 20: og'); : If you don't spe > Is there any consideration behind making hadoop.tables the default? AFAIK h Since we only supported HadoopTables before, I use this as default catalog type, there is no more reasons. You are right, hadoop.catalog gives more than hadoop.tables, so if necessary, I will use hadoop.catalog as default catalog type and modified related files. http://gerrit.cloudera.org:8080/#/c/16446/1/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/16446/1/common/thrift/CatalogObjects.thrift@96 PS1, Line 96: identified > nit: typo Done http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java@244 PS1, Line 244: at getIcebergFile > I find it confusing that IcebergUtil also has a function with the same name Done http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java: http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@71 PS1, Line 71: // Iceberg catalog type key in tblproperties > nit: pls add comment similarly to ICEBERG_FILE_FORMAT above. Done http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java File fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java: http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java@111 PS1, Line 111: return tableParams_.icebergCatalog_; : } : : @ > FeIcebergTable has 2 implementations, this class and IcebergTable. Both hav I add icebergCatalog_ in TableParams in LocalIcebergTable.java, and return 'tableParams_.icebergCatalog_' in getIcebergCatalog(), just like getIcebergTableLocation(). I found that this way is also suitable for getIcebergFileFormat(), I'm not sure to modify in this patch. http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@a93 PS1, Line 93: > As I see this patch also contains some modifications related to Iceberg tab Done http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java File fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java: http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@54 PS1, Line 54: TIcebergCatalo > Using TIcebergCatalog as a param would be easier to understand what is the Done http://gerrit.cloudera.org:8080/#/c/16446/1/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java@63 PS1, Line 63: Preconditions.checkArgument(catalog == TIcebergCatalog.HADOOP_TABLES); > Would worth a Preconditions check in the else branch that icebergCatalog == Done -- To view, visit http://gerrit.cloudera.org:8080/16446 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1893c50a633ca22d4bca6726c9937b026f5d5ef Gerrit-Change-Number: 16446 Gerrit-PatchSet: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Reviewer: wangsheng Gerrit-Comment-Date: Tue, 15 Sep 2020 13:39:06 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10078: Proper codegen for KuduPartitionExpr
Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/16419 ) Change subject: IMPALA-10078: Proper codegen for KuduPartitionExpr .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16419/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16419/3//COMMIT_MSG@12 PS3, Line 12: - TODO: I don't think we have good test coverage on this, I tried to > Does the number of partitions and the number of impalads have to be the sam Would it be possible to write a test to call both the non-llvm and the llvm-code to hash a set of values. If so, then we can just compare the hash results. For v in values hash1 = non_llvm(v); hash2 = llvm(v); assert(hash1 == hash2); -- To view, visit http://gerrit.cloudera.org:8080/16419 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcae34f71b407837e2c5f1b97aa230e490a268df Gerrit-Change-Number: 16419 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 Sep 2020 13:35:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10164: Supporting HadoopCatalog for Iceberg table
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16446 ) Change subject: IMPALA-10164: Supporting HadoopCatalog for Iceberg table .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/7180/ : 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/16446 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic1893c50a633ca22d4bca6726c9937b026f5d5ef Gerrit-Change-Number: 16446 Gerrit-PatchSet: 2 Gerrit-Owner: wangsheng Gerrit-Reviewer: Gabor Kaszab Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 15 Sep 2020 13:13:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-10164: Supporting HadoopCatalog for Iceberg table
wangsheng has uploaded a new patch set (#2). (
http://gerrit.cloudera.org:8080/16446 )
Change subject: IMPALA-10164: Supporting HadoopCatalog for Iceberg table
..
IMPALA-10164: Supporting HadoopCatalog for Iceberg table
This patch mainly realizes creating Iceberg table by HadoopCatalog.
We only supported HadoopTables api before this patch, but now we can
use HadoopCatalog to create Iceberg table. When creating managed table,
we can use SQL like this:
CREATE TABLE default.iceberg_test (
level string,
event_time timestamp,
message string,
)
STORED AS ICEBERG
LOCATION 'hdfs://test-warehouse/hadoop_catalog_test'
TBLPROPERTIES ('iceberg.catalog'='hadoop.catalog');
If you don't specify this property in your SQL, default catalog type is
'hadoop.catalog'. And if you want to create external table, you can use
SQL like this:
CREATE EXTERNAL TABLE default.iceberg_test_external
STORED AS ICEBERG
LOCATION 'hdfs://test-warehouse/hadoop_catalog_test'
TBLPROPERTIES ('iceberg.catalog'='hadoop.catalog',
'iceberg_table_name'='default.iceberg_test');
'iceberg_table_name' is the managed Iceberg table name, just like
'kudu.table_name' when creating external Kudu table. If this property
not been specified in SQL, Impala will use database and table name to
load Iceberg table, which is 'default.iceberg_test_external' in above SQL.
This property cannot be set with managed table.
Testing:
- Create table tests in functional_schema_template.sql
- Iceberg table create test in test_iceberg.py
- Iceberg table query test in test_scanners.py
Change-Id: Ic1893c50a633ca22d4bca6726c9937b026f5d5ef
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeIcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/IcebergTable.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalIcebergTable.java
M fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/util/IcebergUtil.java
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/1-1-bc402da0-b562-4310-9001-06f9b6b0f9ae-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/6-6-d253aefa-65fc-4698-8f26-b155fc965cf6-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/9-9-5d04b016-05e1-43fc-b4a0-0e0df52a5035-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/00017-17-20b92523-c3b9-401d-b429-363c245dbe9c-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/00023-23-c86370cf-10a1-4e49-86dc-b094fe739aa6-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/00027-27-f32f86fa-286f-4cd3-8337-98685c48176d-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/00030-30-b18d2bbc-46a2-4040-a4a8-7488447de3b6-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-08/action=view/00031-31-c9bda250-ed1c-4868-bbf1-f2aad65fa80c-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-09/action=click/4-4-0ed77823-ded1-4a12-9e03-4027cd43966a-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-09/action=click/00014-14-f698d7a4-245f-44d5-8a59-ed511854c8f8-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-09/action=click/00015-15-7c1d5490-91f7-47bd-a3b6-e86caa7fe47d-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-09/action=click/00019-19-d2ef5fcf-4346-421f-b2ef-1f9d55fb4c84-0.parquet
A
testdata/data/iceberg_test/hadoop_catalog/hadoop_catalog_test/functional_parquet/hadoop_catalog_test/data/event_time_hour=2020-01-01-09/action=click/00020-20-a70c64ed-7a99-4f43-ada7-225c
[Impala-ASF-CR] IMPALA-10078: Proper codegen for KuduPartitionExpr
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/16419 ) Change subject: IMPALA-10078: Proper codegen for KuduPartitionExpr .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16419/4/be/src/exec/kudu-util-ir.cc File be/src/exec/kudu-util-ir.cc: http://gerrit.cloudera.org:8080/#/c/16419/4/be/src/exec/kudu-util-ir.cc@54 PS4, Line 54: // TODO: codegen this to eliminate branching on type. > I am not sure if this is still relevant - after this change this function i This function is moved here from kudu-util.cc. This change is not the only place that calls this function (see https://github.com/apache/impala/blob/7b44b351321969fa6a90212f4c9b56c521a85ec4/be/src/exec/kudu-table-sink.cc#L264) and the TODO may still apply there. -- To view, visit http://gerrit.cloudera.org:8080/16419 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcae34f71b407837e2c5f1b97aa230e490a268df Gerrit-Change-Number: 16419 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 Sep 2020 12:51:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10078: Proper codegen for KuduPartitionExpr
Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/16419 ) Change subject: IMPALA-10078: Proper codegen for KuduPartitionExpr .. Patch Set 4: Code-Review+1 (1 comment) About testing: I couldn't come up with anything else than Tim's sanity test. I am ok with testing this one manually, but a test like that would be a nice addittion. http://gerrit.cloudera.org:8080/#/c/16419/4/be/src/exec/kudu-util-ir.cc File be/src/exec/kudu-util-ir.cc: http://gerrit.cloudera.org:8080/#/c/16419/4/be/src/exec/kudu-util-ir.cc@54 PS4, Line 54: // TODO: codegen this to eliminate branching on type. I am not sure if this is still relevant - after this change this function in called with constant col_type argument in the codegend code, so the llvm optimizer has the chance to remove this branch if it inlines the function. -- To view, visit http://gerrit.cloudera.org:8080/16419 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcae34f71b407837e2c5f1b97aa230e490a268df Gerrit-Change-Number: 16419 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 Sep 2020 10:30:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-10078: Proper codegen for KuduPartitionExpr
Daniel Becker has posted comments on this change. ( http://gerrit.cloudera.org:8080/16419 ) Change subject: IMPALA-10078: Proper codegen for KuduPartitionExpr .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/16419/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16419/3//COMMIT_MSG@12 PS3, Line 12: - TODO: I don't think we have good test coverage on this, I tried to > One sanity check I can think of is to write to a hash partitioned table and Does the number of partitions and the number of impalads have to be the same for this to work? Am I right that the assumption here is that the hash function is uniform and if the implementation of partitioning is correct, this uniformity is preserved? -- To view, visit http://gerrit.cloudera.org:8080/16419 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcae34f71b407837e2c5f1b97aa230e490a268df Gerrit-Change-Number: 16419 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Becker Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Daniel Becker Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Qifan Chen Gerrit-Reviewer: Thomas Tauber-Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 15 Sep 2020 08:23:47 + Gerrit-HasComments: Yes
