[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#3). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 145 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/3 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13400 )
Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS
integration
..
Patch Set 3:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1439
PS2, Line 1439: // The Kudu tables in the HMS should have been dropped at
this point
: // with the Hive Metastore integrat
> Shouldn't this be true regardless of HMS integration? Isn't that what dropT
Sorry for the confusion, I mean the Kudu table in the HMS should be deleted at
this pointed.
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1508
PS2, Line 1508: KuduCatalogOpExecutor.dropTable(msTable, /*if exists*/ true);
> Does it wait for the table to be actually dropped from Kudu?
Are you referring to wait for the table to be actually dropped in the HMS when
HMS integration is enabled? I think this is not necessary. The table should be
dropped in the HMS if dropping the table in Kudu returns successfully.
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1512
PS2, Line 1512: private boolean isKuduHmsIntegrationEnab
> The CatalogOpExecutor class isn't Kudu-specific; this should probably be na
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1613
PS2, Line 1613: // When Kudu's HMS integration is enabled, Kudu will drop
the managed table
> Please add a comment explaining the rationale of this condition, eg
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1616
PS2, Line 1616: if (!isManagedKuduTable ||
!isKuduHmsIntegrationEnabled(msTbl)) {
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py
File tests/custom_cluster/test_kudu.py:
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@124
PS2, Line 124:
> While adding this attribute, could you justify on the reasoning in the doc
My understanding is that all tests in under custom_cluster dir should run
serially because all of them requires restart certain components with
customized configurations.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@133
PS2, Line 133: def teardown_class(cls):
> I'm not sure this test scenario requires hybrid clock to be present. Why w
Yeah, I think this is actually not needed.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@138
PS2, Line 138:
> What if db_name is 'default' ?
I think the unique_cursor ensured the db name is unique.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@150
PS2, Line 150: """
> Ditto for the reasoning to skip the test if no hybrid clock is around: why
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@153
PS2, Line 153: elf.get_kudu_table_base_name(kudu_table.
> nit: move this into a separate sentence for easier comprehension and maybe
Done
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@155
PS2, Line 155: table_n
> What if db_name is 'default'?
Same as above.
http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@168
PS2, Line 168: assert ["", "storage_handler", "org.apache.kudu.hive
> Interesting, in case of Kudu+HMS systests which I ran on recent CDH6.x this
I don't quite understand why you encountered scenario that after dropping the
table in Impala, the kudu table is dropped after some time. Because drop table
in Impala will call Kudu drop table API explicitly which should be a sync call.
Would you mind trying again with the latest bits and with my patches included?
Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/13400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc
Gerrit-Change-Number: 13400
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 03 Jun 2019 07:11:05 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/13400/3/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/3/tests/custom_cluster/test_kudu.py@283 PS3, Line 283: flake8: W292 no newline at end of file -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 07:11:35 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 3: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4388/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 07:11:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3480/ : 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/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 07:52:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8369: Add HIVE MAJOR VERSION section to EE tests + some fixes
Csaba Ringhofer has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13472 ) Change subject: IMPALA-8369: Add HIVE_MAJOR_VERSION section to EE tests + some fixes .. IMPALA-8369: Add HIVE_MAJOR_VERSION section to EE tests + some fixes Fixed tests with Hive3: test_scanners.py - test_scan_truncated_file_empty (exhaustive): Added REFRESH after Hive INSERT OVERWRITE. The test worked in Hive2 only because there was an empty file with the same name as before the overwrite. test_ddl.py - test_alter_table: A Hive3 regression broke some tests + caused the dropping of the test database to hang. These tests are skipped for now, the Hive side fix is tracked in HIVE-21806. Change-Id: I4c3cff05ed7080b655b6af64ea09c0691e7dd931 Reviewed-on: http://gerrit.cloudera.org:8080/13472 Reviewed-by: Zoltan Borok-Nagy Tested-by: Impala Public Jenkins --- M testdata/workloads/functional-query/queries/QueryTest/alter-table.test M tests/common/impala_test_suite.py M tests/query_test/test_scanners.py M tests/util/test_file_parser.py 4 files changed, 29 insertions(+), 1 deletion(-) Approvals: Zoltan Borok-Nagy: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13472 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4c3cff05ed7080b655b6af64ea09c0691e7dd931 Gerrit-Change-Number: 13472 Gerrit-PatchSet: 3 Gerrit-Owner: Csaba Ringhofer Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8369: Fix HMS integration tests for Hive 3
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13475 ) Change subject: IMPALA-8369: Fix HMS integration tests for Hive 3 .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13475/4/tests/metadata/test_hms_integration.py File tests/metadata/test_hms_integration.py: http://gerrit.cloudera.org:8080/#/c/13475/4/tests/metadata/test_hms_integration.py@205 PS4, Line 205: parse_hive3_describe_formatted_output > The new format of the the command in hive looks very weird. I wonder if its It seems also an intentional change and a bug as well. I looked at the test files of the 'describe formatted' command: https://github.com/apache/hive/blob/10b6d70da1442cccf533bc97f56a622ec9f39661/ql/src/test/results/clientpositive/describe_table.q.out#L201-L216 So it seems to me that Hive switched to this vertical output format. On the other hand, the output Hive really produces has lots of garbage. Interestingly, on Hive cwiki there is an example to this strange output at https://cwiki.apache.org/confluence/display/Hive/StatsDev#StatsDev-ExistingTables , search for "the output would look like this". But I still think that it's a bug, so I opened HIVE-21824. Anyhow, this function should parse both the intended vertical format and the buggy output as well. -- To view, visit http://gerrit.cloudera.org:8080/13475 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I51c91c7fc706175295b78abaacf47a86156714ce Gerrit-Change-Number: 13475 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Mon, 03 Jun 2019 12:38:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 3: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4388/ -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 12:45:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.
Andrew Sherman has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13307 ) Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission Controller. .. IMPALA-8536: Add Scalable Pool Configuration to Admission Controller. Add 3 configuration parameters to Admission Controller that scale with the number of hosts in the resource pool. These parameters are specified to the Impalad through the -llama_site_path flag which points to a Llama XML configuration file. The new configuration parameters are: + Max Running Queries Multiple - this floating point number is multiplied by the current total number of executors at runtime to give the maximum number of concurrently running queries allowed in the pool. This calculation is rounded up to the nearest integer so the result will always be at least one as long as the parameter is non-zero. + Max Queued Queries Multiple - this floating point number is multiplied by the current total number of executors at runtime to give the maximum number of queries that can be queued in the pool. This calculation is rounded up to the nearest integer so the result will always be at least one as long as the parameter is non-zero. + Max Memory Multiple - this number of bytes is multiplied by the current total number of executors at runtime to give the maximum memory available across the cluster for the pool. If any of these parameters have zero value then they will be ignored. In this case the corresponding non-scalable parameters will be used, if they are set. The new parameters are exposed through the webui. At various points in the code Admission Controller looks at the Pool Config objects to find non-scalable parameters such as the max number of queries that can run in the pool. These access have been encapsulated in functions that return the scalable version of the configuration value if the new scalable parameters are being used. Diagnostic messages are enhanced to show the origin of the encapsulated parameters. TESTING All end-to-end tests are running clean with ASAN. The unit test admission-controller-test.cc has been expanded to test the newly added code. Added an end-to-end test that adds and removes Impalads from a minicluster. Change-Id: If47508728124076f3b9200c27cffc989f7a4f188 --- M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/admission-controller.h M common/thrift/ImpalaInternalService.thrift M common/thrift/generate_metrics.py M common/thrift/metrics.json M fe/src/main/java/org/apache/impala/util/RequestPoolService.java M fe/src/test/java/org/apache/impala/util/TestRequestPoolService.java M fe/src/test/resources/fair-scheduler-test.xml M fe/src/test/resources/fair-scheduler-test2.xml M fe/src/test/resources/llama-site-test.xml M fe/src/test/resources/llama-site-test2.xml M testdata/workloads/functional-query/queries/QueryTest/admission-max-min-mem-limits.test M testdata/workloads/functional-query/queries/QueryTest/admission-reject-mem-estimate.test M testdata/workloads/functional-query/queries/QueryTest/admission-reject-min-reservation.test M tests/common/impala_cluster.py M tests/custom_cluster/test_admission_controller.py M tests/webserver/test_web_pages.py M www/admission_controller.tmpl 19 files changed, 1,193 insertions(+), 270 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/07/13307/6 -- To view, visit http://gerrit.cloudera.org:8080/13307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188 Gerrit-Change-Number: 13307 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#4). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 146 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/4 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@284 PS4, Line 284: flake8: W292 no newline at end of file -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 17:02:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13500 Change subject: Disable cumstom cluster/service FE tests on S3 .. Disable cumstom cluster/service FE tests on S3 This patch disables the cumstom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 7 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/1 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4389/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 17:14:42 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4390/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 17:15:00 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 17:16:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13319 ) Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. Patch Set 5: Code-Review+2 Thanks! -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 17:17:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin
Thomas Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13319 ) Change subject: IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin .. IMPALA-8503: allow the Hive Metastore to start with kudu-hive plugin This patch allows to start the Hive Metasotre with Kudu plugin which is required for enabling Kudu's integration with the HMS. The Kudu plugin is downloaded and extracted from native-toolchain S3 bucket. Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Reviewed-on: http://gerrit.cloudera.org:8080/13319 Tested-by: Impala Public Jenkins Reviewed-by: Thomas Marshall --- M bin/bootstrap_toolchain.py M bin/impala-config.sh M fe/src/test/resources/hive-site.xml.py M impala-parent/pom.xml M testdata/bin/run-hive-server.sh 5 files changed, 22 insertions(+), 5 deletions(-) Approvals: Impala Public Jenkins: Verified Thomas Marshall: Looks good to me, approved -- To view, visit http://gerrit.cloudera.org:8080/13319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I4bd1488ced51840ec986d29ed371e26168abcc76 Gerrit-Change-Number: 13319 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8509. Lazily evaluate LOAD sections during data load
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13252 ) Change subject: IMPALA-8509. Lazily evaluate LOAD sections during data load .. Patch Set 4: yea, Joe and I looked at it and it seems like there's still some dependency where earlier-listed tables were expecting those files to have been loaded to HDFS, but they weren't, or somesuch. Need to circle back on this one when I get some free time. -- To view, visit http://gerrit.cloudera.org:8080/13252 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifc64bb5cac4fda675607672329c04c5caf810d99 Gerrit-Change-Number: 13252 Gerrit-PatchSet: 4 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Tim Armstrong Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Mon, 03 Jun 2019 17:21:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13307 ) Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission Controller. .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3481/ : 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/13307 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188 Gerrit-Change-Number: 13307 Gerrit-PatchSet: 6 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Bikramjeet Vig Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 17:29:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/2/tests/custom_cluster/test_kudu.py@168 PS2, Line 168: assert not kudu_client.table_exists(kudu_table.name) > I don't quite understand why you encountered scenario that after dropping t I'll try to remove assert_eventually() from the systest and verify how it works. As of now, I cannot install latest bits on my cluster. -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 17:33:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8564: Add table/view create time in the lineage graph
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13399 ) Change subject: IMPALA-8564: Add table/view create time in the lineage graph .. Patch Set 18: (1 comment) http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java File fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java: http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java@787 PS18, Line 787:* This is only for testing. It does not check for user and timestamp fields. since it's only for testing, could we change this to not override Object.equals, but instead be named something like 'equalForTests()'? Given that .equals() is called implicitly in a lot of cases, this seems like a ticking time bomb that someone uses it and gets confused by the semantics. (same goes for the above) -- To view, visit http://gerrit.cloudera.org:8080/13399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82 Gerrit-Change-Number: 13399 Gerrit-PatchSet: 18 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 03 Jun 2019 17:37:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3482/ : 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/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 17:42:31 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3483/ : 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/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 17:53:25 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13500 )
Change subject: Disable cumstom cluster/service FE tests on S3
..
Patch Set 1: Code-Review-1
(2 comments)
I think the mismatched pushd/popd will cause problems in the S3 case.
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@250
PS1, Line 250: if [[ "${TARGET_FILESYSTEM}" != "s3" ]]; then
I think it would be better to check if FE_TEST is true. I think we want to skip
it for other similar filesystems, e.g. ADLS.
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@257
PS1, Line 257: popd
pushd and popd are now mismatched.
--
To view, visit http://gerrit.cloudera.org:8080/13500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245
Gerrit-Change-Number: 13500
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 03 Jun 2019 17:54:53 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py@1130 PS5, Line 1130: @SkipIfKudu.hms_integration_enabled I still think that a lot of the tests in this file and elsewhere that have been skipped don't need to be - eg. I'm not sure why this one wouldn't work as is with hms integration enabled. It also occurs to me - as written, this 'skip' won't ever actually be triggered in Impala's normal test runs, as we'll always run these tests against the default minicluster config, which doesn't enable HMS integration. Of course, even in that case people may use these tests to run in other situations, eg. there have been efforts to run them against real clusters, so it still makes sense to include the 'skip' for any tests that genuinely won't ever work with hms integration turned on. I'm wondering though if it would be worth the effort and testing time to parameterize these tests so that they all run both with and without hms integration enabled, eg. by having custom_cluster/TestKuduHMSIntegration inherent from some of these classes? A lot of the tests in this file really won't be impacted by the integration (eg. the scan tests) and it would be basically wasted test time to run them in both configurations, but a lot of them are affected (eg. this test is one I would definitely want to know works with HMS integration turned on) One question I have is what's Kudu plan for how this evolves in future version - i.e. is the non-HMS integrated functionality going to be aggressively deprecated, such that Impala will probably want to just make hms integration turned on the default config in our minicluster, or is the plan to maintain both code paths for awhile? Anyways, I think this patch has fairly good coverage as is, and we can probably just file a JIRA to clean some of this up later. -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 18:09:57 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/13307 ) Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission Controller. .. Patch Set 6: (15 comments) http://gerrit.cloudera.org:8080/#/c/13307/5/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/13307/5/be/src/scheduling/admission-controller.h@663 PS5, Line 663: Use a nit: Uses a http://gerrit.cloudera.org:8080/#/c/13307/5/be/src/scheduling/admission-controller.h@664 PS5, Line 664: This is based on the : /// max_requests_estimate limit and the current queue size. We will attempt to : /// dequeue up to this number of requests until reaching the per-pool memory limit nit: not sure if this detail is relevant there. maybe remove this? http://gerrit.cloudera.org:8080/#/c/13307/3/be/src/scheduling/admission-controller.h File be/src/scheduling/admission-controller.h: http://gerrit.cloudera.org:8080/#/c/13307/3/be/src/scheduling/admission-controller.h@603 PS3, Line 603: _size_snapshot, > The ClusterMembershipMgr that will come in IMPALA-8460 has a method GetSnap Not a common term but just reviewing IMPALA-8460 drilled into me that the notion that a snapshot is like a snapshot of some (meta)data. how about cluster_snapshot_size? http://gerrit.cloudera.org:8080/#/c/13307/3/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/13307/3/be/src/scheduling/admission-controller.cc@1396 PS3, Line 1396: > I originally used the cluster size from QuerySchedule but hit problems with I thought about this more and I think not counting the quiescing backends would be the better alternative since in this case the queries scheduled to run on those backends would not hold up the rest of the queue and in case queries scheduled on lesser num of backends get overadmitted, we can consider those an extension of the case where a query is only scheduled on a subset of backends (another eg would be query only scheduled on coord). http://gerrit.cloudera.org:8080/#/c/13307/3/be/src/scheduling/admission-controller.cc@1444 PS3, Line 1444: rn > should be OR (||) http://gerrit.cloudera.org:8080/#/c/13307/5/be/src/scheduling/admission-controller.cc File be/src/scheduling/admission-controller.cc: http://gerrit.cloudera.org:8080/#/c/13307/5/be/src/scheduling/admission-controller.cc@1051 PS5, Line 1051: absolute limit nit: outdated comment http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py File tests/custom_cluster/test_admission_controller.py: http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@994 PS5, Line 994: Impalada nit: Impalads http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1002 PS5, Line 1002: def check_admission_by_memory(self, expected_admission, expected_rejection_reason=None): nit: by convention, we usually prefix a helper method with a double underscore ( __ ) http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1003 PS5, Line 1003: query = "select * from functional.alltypesagg order by int_col limit 1" nit: add method comment http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1003 PS5, Line 1003: query = "select * from functional.alltypesagg order by int_col limit 1" might be useful to set the mem_limit too to make this less flaky in case the estimates change in the future. http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1015 PS5, Line 1015: assert expected_rejection_reason in rejected_reasons[0] nit: print the profile if this assert fails for easy debugging http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1016 PS5, Line 1016: yybecause nit: perhaps you forgot to remove this line? http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1025 PS5, Line 1025: Run some queries, find how many were admitted, queued or rejected, and then compare nit: maybe mention that this is to verify if the admission controller correctly enforces query count limits http://gerrit.cloudera.org:8080/#/c/13307/5/tests/custom_cluster/test_admission_controller.py@1037 PS5, Line 1037: assert (expected_num_admitted + expected_num_queued + : expected_num_rejected) == NUM_QUERIES it seems like we can calculate all these expected values from the num of impalads in the cluster, maybe that is the only variable we need to pass to this methods since the logic for expected values is incorporated in this method itself (as the request_pool is constant) . http://gerrit.cloudera.org:8080/#/
[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13425 )
Change subject: IMPALA-8542. Add an access trace for the data cache
..
Patch Set 2:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache-test.cc
File be/src/runtime/io/data-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache-test.cc@504
PS2, Line 504: expect_misses);
> nit: indent 4 spaces here and below
see above
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.h
File be/src/runtime/io/data-cache.h:
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.h@269
PS2, Line 269: /// The file name used for the access trace.
> nit: newline before this one
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc
File be/src/runtime/io/data-cache.cc:
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@88
PS2, Line 88: c
> nit: Capitalize first word
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@97
PS2, Line 97: const char* DataCache::Partition::TRACE_FILE_NAME =
"impala-cache-trace.txt";
> Should we include the partition name in the trace to make it easier to copy
I think concatenation should be fine, or if we ask users to collect them, we
would provide a script that can do the right thing (eg using tar to preserve
the original paths).
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@159
PS2, Line 159: explicit Tracer(string path)
> nit: single line
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@165
PS2, Line 165: logger_->Stop();
> nit: single line
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@413
PS2, Line 413: return UNALIGNED_LOAD64(key_.data() + key_.size() -
sizeof(int64_t) * 2);
> Can you add a comment to point out that these hard-code the key layout? Sho
Rearranged the key and added some constants to make this clearer. I didn't end
up adding the extra name_len field since it seemed a bit wasteful of memory and
not much gain.
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@544
PS2, Line 544: if (tracer_) tracer_->Trace(Tracer::MISS, cache_key,
bytes_to_read, /*entry_len=*/-1);
> nit: the rest of the file uses explicit pointer comparison (see above), sho
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@656
PS2, Line 656: if (tracer_) tracer_->Trace(Tracer::STORE_FAILED_BUSY,
cache_key,
> style: Enclose multiple lines in {}, indent 4 spaces
Enclosed in {} but the style guide says to align the parameters on the second
line with the parameters on the first line:
https://google.github.io/styleguide/cppguide.html#Function_Calls :
bool result = DoSomething(averyveryveryverylongargument1,
argument2, argument3);
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@860
PS2, Line 860: S!
> Would it make automatic parsing easier if this was a single character, e.g.
Done
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@871
PS2, Line 871: C
> Can this be a static assert?
I don't think CalcualteBase64EscapedLen is constexpr (it's defined in the .cc,
not the header)
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@874
PS2, Line 874:sizeof(hash), buf, buf_len);
> nit: indent 4 spaces
see above, google style guide disagrees
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@903
PS2, Line 903:
> nit: extra empty line
Done
--
To view, visit http://gerrit.cloudera.org:8080/13425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 2
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 03 Jun 2019 18:33:58 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache
Hello Michael Ho, Lars Volker, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13425 to look at the new patch set (#3). Change subject: IMPALA-8542. Add an access trace for the data cache .. IMPALA-8542. Add an access trace for the data cache This adds a relatively simple JSON-formatted access trace for the data cache feature. Each partition stores a trace file named 'trace.txt', with each line representing a hit, miss, or store into the cache. The trace is collected using the kudu::AsyncLogger class which handles buffering and deferring the actual IO to a background thread. By default, the full cache key info is written to the trace (including the file paths), but a flag can enable anonymization (128-bit city-hashing) of the file names in case any user would like to capture a trace to be shared publically without divulging their table names. Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c --- M be/src/runtime/io/data-cache-test.cc M be/src/runtime/io/data-cache.cc M be/src/runtime/io/data-cache.h 3 files changed, 293 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/25/13425/3 -- To view, visit http://gerrit.cloudera.org:8080/13425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c Gerrit-Change-Number: 13425 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7957: Fix slot equivalences may be enforced multiple times
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13051 ) Change subject: IMPALA-7957: Fix slot equivalences may be enforced multiple times .. Patch Set 5: Code-Review+2 Sorry for the slow turnaround. Greatly appreciate your work on this - thank you for fixing such a tricky bug. -- To view, visit http://gerrit.cloudera.org:8080/13051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2d5d8149b217e18ebae61e136848162503653e Gerrit-Change-Number: 13051 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 18:38:05 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8564: Add table/view create time in the lineage graph
Fredy Wijaya has uploaded a new patch set (#19). (
http://gerrit.cloudera.org:8080/13399 )
Change subject: IMPALA-8564: Add table/view create time in the lineage graph
..
IMPALA-8564: Add table/view create time in the lineage graph
This patch adds table/view create time in the lineage graph. This is
needed for Impala/Atlas integration. See ATLAS-3080.
Below is an example of the updated lineage graph.
{
"queryText":"create table lineage_test_tbl as select int_col, tinyint_col
from functional.alltypes",
"queryId":"0:0",
"hash":"407f23b24758ffcb2ac445b9703f5c44",
"user":"dummy_user",
"timestamp":1547867921,
"edges":[
{
"sources":[
1
],
"targets":[
0
],
"edgeType":"PROJECTION"
},
{
"sources":[
3
],
"targets":[
2
],
"edgeType":"PROJECTION"
}
],
"vertices":[
{
"id":0,
"vertexType":"COLUMN",
"vertexId":"int_col",
"metadata":{
"tableName":"default.lineage_test_tbl",
"tableCreateTime":1559151337
}
},
{
"id":1,
"vertexType":"COLUMN",
"vertexId":"functional.alltypes.int_col",
"metadata":{
"tableName":"functional.alltypes",
"tableCreateTime":1559151317
}
},
{
"id":2,
"vertexType":"COLUMN",
"vertexId":"tinyint_col",
"metadata":{
"tableName":"default.lineage_test_tbl",
"tableCreateTime":1559151337
}
},
{
"id":3,
"vertexType":"COLUMN",
"vertexId":"functional.alltypes.tinyint_col",
"metadata":{
"tableName":"functional.alltypes",
"tableCreateTime":1559151317
}
}
]
}
Testing:
- Updated lineage tests in PlannerTest
- Updated test_lineage.py
- Ran all FE tests
Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82
---
M be/src/service/client-request-state.h
M be/src/service/frontend.cc
M be/src/service/impala-server.cc
M be/src/util/lineage-util.h
M common/thrift/CatalogService.thrift
M common/thrift/LineageGraph.thrift
M fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java
M fe/src/main/java/org/apache/impala/analysis/CreateOrAlterViewStmtBase.java
M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
M fe/src/main/java/org/apache/impala/service/JniFrontend.java
M fe/src/test/java/org/apache/impala/planner/PlannerTestBase.java
M testdata/workloads/functional-planner/queries/PlannerTest/lineage.test
M tests/custom_cluster/test_lineage.py
15 files changed, 2,191 insertions(+), 627 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/13399/19
--
To view, visit http://gerrit.cloudera.org:8080/13399
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82
Gerrit-Change-Number: 13399
Gerrit-PatchSet: 19
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Anonymous Coward (498)
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8564: Add table/view create time in the lineage graph
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13399 ) Change subject: IMPALA-8564: Add table/view create time in the lineage graph .. Patch Set 19: (1 comment) http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java File fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java: http://gerrit.cloudera.org:8080/#/c/13399/18/fe/src/main/java/org/apache/impala/analysis/ColumnLineageGraph.java@787 PS18, Line 787: Set vertices = new HashSet<>(); > since it's only for testing, could we change this to not override Object.eq You're right. It's actually better to have proper equals + hashCode plus a specific equalsForTests especially we store the Vertex in a Set which requires proper equals + hashCode. Done. -- To view, visit http://gerrit.cloudera.org:8080/13399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82 Gerrit-Change-Number: 13399 Gerrit-PatchSet: 19 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 03 Jun 2019 18:46:22 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7957: Fix slot equivalences may be enforced multiple times
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13051 ) Change subject: IMPALA-7957: Fix slot equivalences may be enforced multiple times .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4391/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2d5d8149b217e18ebae61e136848162503653e Gerrit-Change-Number: 13051 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 18:47:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13491 to look at the new patch set (#4). Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h .. IMPALA-8578: part 1: reduce dependencies on *metrics.h Before this patch there were 100s of compilation units that pulled in metrics.h and the significant amount of code in that header. It was painfuly slow to recompile after changes to that file. The patch reduces that significantly and mostly eliminates transitive inclusions via other headers. * Add metric-defs.h with forward declarations needed to have pointers to the various classes. * Update headers to use metric-defs.h and move includes of *metrics.h to the .cc files. * Add includes, etc to fix compilation errors where files depended on transitively-included headers from *metrics.h This shaved about 30s off the build time on Jenkins - about a 4% speedup. I didn't end up removing anything from the headers - that is a bit more work since most of the classes are templatized and need to be explicitly instantiated in .cc files if functions are not all defined in the headers. Testing: Ran a core build Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb --- M be/src/catalog/catalog-server.cc M be/src/catalog/catalog-server.h M be/src/exec/exec-node.h M be/src/exec/hash-table.h M be/src/exec/hdfs-orc-scanner.h M be/src/exec/hdfs-scan-node-base.cc M be/src/exec/hdfs-table-sink.cc M be/src/exec/parquet/hdfs-parquet-scanner.cc M be/src/exec/parquet/hdfs-parquet-table-writer.cc M be/src/exec/parquet/parquet-column-readers.cc M be/src/exec/plan-root-sink.cc M be/src/exec/scan-node.cc M be/src/experiments/data-provider-test.cc M be/src/experiments/tuple-splitter-test.cc M be/src/rpc/TAcceptQueueServer.cpp M be/src/rpc/TAcceptQueueServer.h M be/src/rpc/thrift-server.h M be/src/rpc/thrift-util.cc M be/src/runtime/buffered-tuple-stream.cc M be/src/runtime/bufferpool/buffer-allocator-test.cc M be/src/runtime/bufferpool/reservation-tracker.cc M be/src/runtime/client-cache.cc M be/src/runtime/client-cache.h M be/src/runtime/coordinator-backend-state.cc M be/src/runtime/initial-reservations.cc M be/src/runtime/io/data-cache.cc M be/src/runtime/io/disk-io-mgr.cc M be/src/runtime/io/handle-cache.h M be/src/runtime/io/handle-cache.inline.h M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/io/local-file-reader.cc M be/src/runtime/krpc-data-stream-mgr.cc M be/src/runtime/krpc-data-stream-mgr.h M be/src/runtime/mem-pool.cc M be/src/runtime/mem-tracker.cc M be/src/runtime/mem-tracker.h M be/src/runtime/query-exec-mgr.cc M be/src/runtime/query-state.cc M be/src/runtime/sorter.cc M be/src/runtime/tmp-file-mgr-test.cc M be/src/runtime/tmp-file-mgr.cc M be/src/runtime/tmp-file-mgr.h M be/src/scheduling/request-pool-service.cc M be/src/scheduling/request-pool-service.h M be/src/scheduling/scheduler.h M be/src/service/client-request-state.cc M be/src/service/impala-beeswax-server.cc M be/src/service/impala-hs2-server.cc M be/src/service/impala-server.cc M be/src/service/impalad-main.cc M be/src/service/session-expiry-test.cc M be/src/statestore/statestore-subscriber.cc M be/src/statestore/statestore-subscriber.h M be/src/statestore/statestore.cc M be/src/statestore/statestore.h M be/src/util/collection-metrics.h M be/src/util/impalad-metrics.cc M be/src/util/impalad-metrics.h A be/src/util/metric-defs.h M be/src/util/metrics.h 60 files changed, 160 insertions(+), 61 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/13491/4 -- To view, visit http://gerrit.cloudera.org:8080/13491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb Gerrit-Change-Number: 13491 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py File tests/query_test/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13375/5/tests/query_test/test_kudu.py@1130 PS5, Line 1130: @SkipIfKudu.hms_integration_enabled > I still think that a lot of the tests in this file and elsewhere that have The tests that are skipped when HMS integration enabled for mainly two reasons: 1) the table name has "impala::" prefix hardcoded. 2) using external table, which could has name collision with managed table created by Kudu. Yeah, I think it makes sense to parameterize some tests so that they all run both with and without hms integration enabled. (I was trying to do it, however don't find a good way to achieve it yet). I can file a JIRA to track this. Non HMS integrated mode is not something going to be aggressively deprecated. So we are planning to maintain both mode for awhile. -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 19:15:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13425 ) Change subject: IMPALA-8542. Add an access trace for the data cache .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3484/ : 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/13425 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c Gerrit-Change-Number: 13425 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 03 Jun 2019 19:18:43 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hello Thomas Marshall, Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13500 to look at the new patch set (#2). Change subject: Disable cumstom cluster/service FE tests on S3 .. Disable cumstom cluster/service FE tests on S3 This patch disables the cumstom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/2 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13500 )
Change subject: Disable cumstom cluster/service FE tests on S3
..
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh
File bin/run-all-tests.sh:
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@250
PS1, Line 250: if [[ "$FE_TEST" == true && "${TARGET_FILESYSTEM}" != "s3"
]]; then
> I think it would be better to check if FE_TEST is true. I think we want to
Done
http://gerrit.cloudera.org:8080/#/c/13500/1/bin/run-all-tests.sh@257
PS1, Line 257: fi
> pushd and popd are now mismatched.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13500
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245
Gerrit-Change-Number: 13500
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 03 Jun 2019 19:24:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8564: Add table/view create time in the lineage graph
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13399 ) Change subject: IMPALA-8564: Add table/view create time in the lineage graph .. Patch Set 19: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3485/ : 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/13399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If4f578d7b299a76c30323b10a883ba32f8713d82 Gerrit-Change-Number: 13399 Gerrit-PatchSet: 19 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Anonymous Coward (498) Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 03 Jun 2019 19:26:36 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 2: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/3487/ : Initial code review checks failed. See linked job for details on the failure. -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 19:39:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8578: part 1: reduce dependencies on *metrics.h
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13491 ) Change subject: IMPALA-8578: part 1: reduce dependencies on *metrics.h .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3486/ : 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/13491 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie2942366cab5421f2db7c27e7da712ea6f775fdb Gerrit-Change-Number: 13491 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 03 Jun 2019 19:43:57 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#6). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 306 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/6 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache
Lars Volker has posted comments on this change. (
http://gerrit.cloudera.org:8080/13425 )
Change subject: IMPALA-8542. Add an access trace for the data cache
..
Patch Set 3: Code-Review+1
(1 comment)
LGTM, I'd be curious how others feel about the indentation, but I don't intend
to hold the change up.
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc
File be/src/runtime/io/data-cache.cc:
http://gerrit.cloudera.org:8080/#/c/13425/2/be/src/runtime/io/data-cache.cc@656
PS2, Line 656: // Limit the write concurrency to avoid blocking the caller
(which could be calling
> Enclosed in {} but the style guide says to align the parameters on the seco
I think we usually go with "start the arguments on a new line indented by four
spaces and continue at that 4 space indent." throughout the codebase, though
historically we've also started at the same line as the function call and then
continued with 4 spaces, e.g. in L227, L510, and others.
I'm personally used to our style and therefore am in favor of consistency over
guidelines
(https://google.github.io/styleguide/cppguide.html#Existing_Non-conformant_Code
).
--
To view, visit http://gerrit.cloudera.org:8080/13425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 03 Jun 2019 20:20:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3488/ : 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/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 20:37:32 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4392/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 20:57:17 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 20:56:45 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable cumstom cluster/service FE tests on S3
Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable cumstom cluster/service FE tests on S3 .. Patch Set 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@7 PS2, Line 7: cumstom typo http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@9 PS2, Line 9: cumstom typo -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 21:20:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Hello Thomas Marshall, Lars Volker, Tim Armstrong, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13500 to look at the new patch set (#3). Change subject: Disable custom cluster/service FE tests on S3 .. Disable custom cluster/service FE tests on S3 This patch disables the custom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 --- M bin/run-all-tests.sh 1 file changed, 8 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13500/3 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Patch Set 3: (2 comments) http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@7 PS2, Line 7: custom > typo Done http://gerrit.cloudera.org:8080/#/c/13500/2//COMMIT_MSG@9 PS2, Line 9: custom > typo Done -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 21:44:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-5031: signed overflow is undefined behavior
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13437 ) Change subject: IMPALA-5031: signed overflow is undefined behavior .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13437/1/be/src/util/arithmetic-util.h File be/src/util/arithmetic-util.h: http://gerrit.cloudera.org:8080/#/c/13437/1/be/src/util/arithmetic-util.h@117 PS1, Line 117: // This operator makes that behavior defined by doing it in the unsigned domain. Maybe document exactly what this implies - my understand is that negating the minimum value of T results in exactly the same value. -- To view, visit http://gerrit.cloudera.org:8080/13437 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I73dd6802ec1023275d09a99a2950f3558313fc8e Gerrit-Change-Number: 13437 Gerrit-PatchSet: 1 Gerrit-Owner: Jim Apple Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 21:58:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8553,IMPALA-8552: fix checks for remote cluster
David Knupp has posted comments on this change. ( http://gerrit.cloudera.org:8080/13386 ) Change subject: IMPALA-8553,IMPALA-8552: fix checks for remote cluster .. Patch Set 4: > Patch Set 4: Verified-1 > > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4371/ Hmm. Seems unrelated. Has this test been failing? EMESSAGE: AnalysisException: Table does not exist: test_chars_tmp_tables_ea7bc735.test_char_nulls -- To view, visit http://gerrit.cloudera.org:8080/13386 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa6b2a1391f53121d3d7c00c5cf0a57590899ce4 Gerrit-Change-Number: 13386 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 22:08:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8603: remove confusing logging from custom cluster tests
Thomas Marshall has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13502 Change subject: IMPALA-8603: remove confusing logging from custom cluster tests .. IMPALA-8603: remove confusing logging from custom cluster tests IMPALA-1653 added explicit closing of the HS2 session that is created by ImpalaTestSuite during teardown. For the custom cluster tests, this sometimes fails, either because the impalad was shutdown and is no longer reachable or because it was restarted and the new impalad doesn't know about the session that was created before restart. Currently, when these errors occur we log them. However, this is confusing because several custom cluster tests now log errors even though they succeed. This patch changes this to simply ignore the errors. One concern is that a test may hit a genuine error here and if so we won't have the logging for it. If this happens, though, it should generally be possible for a dev to repro the error with the logging enabled. Change-Id: I96144fdbe6cc9bf0f01a9951420ee6f281fa6649 --- M tests/common/impala_connection.py 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/02/13502/1 -- To view, visit http://gerrit.cloudera.org:8080/13502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I96144fdbe6cc9bf0f01a9951420ee6f281fa6649 Gerrit-Change-Number: 13502 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall
[Impala-ASF-CR] IMPALA-8553,IMPALA-8552: fix checks for remote cluster
Tim Armstrong has removed a vote on this change. Change subject: IMPALA-8553,IMPALA-8552: fix checks for remote cluster .. Removed Verified-1 by Impala Public Jenkins -- To view, visit http://gerrit.cloudera.org:8080/13386 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: Ifa6b2a1391f53121d3d7c00c5cf0a57590899ce4 Gerrit-Change-Number: 13386 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8553,IMPALA-8552: fix checks for remote cluster
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13386 ) Change subject: IMPALA-8553,IMPALA-8552: fix checks for remote cluster .. Patch Set 4: Yeah that was caused by https://issues.apache.org/jira/browse/IMPALA-8567 -- To view, visit http://gerrit.cloudera.org:8080/13386 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifa6b2a1391f53121d3d7c00c5cf0a57590899ce4 Gerrit-Change-Number: 13386 Gerrit-PatchSet: 4 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: David Knupp Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 22:21:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 6: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 22:22:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 6: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4394/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 22:23:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13465 ) Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4395/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 22:37:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8542. Add an access trace for the data cache
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/13425 )
Change subject: IMPALA-8542. Add an access trace for the data cache
..
Patch Set 3:
(8 comments)
LGTM. Mostly nits.
http://gerrit.cloudera.org:8080/#/c/13425/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13425/3//COMMIT_MSG@10
PS3, Line 10: trace.txt
impala-cache-trace.txt ?
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc
File be/src/runtime/io/data-cache.cc:
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@88
PS3, Line 88: "(Advanced) Collect a trace of all lookups in the
data cache.");
nit: the indent here and other places in this change seems to be different from
the existing code. We usually indent 4 for line continuation. Seems more
consistent to follow the indent 4 convention.
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@98
PS3, Line 98: impala-cache-trace.txt
The data cache by default removes all files with names containing prefix
CACHE_FILE_PREFIX at startup. I suppose we want to keep the trace files across
restart to allow more flexibility for experiment, right ? In other words, it's
up to users to remove the trace files.
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@116
PS3, Line 116: if (file_) Flush();
Does it make sense to also call file_->Close() here or does the d'tor take care
of it ?
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@148
PS3, Line 148:
nit: unnecessary blank line.
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@186
PS3, Line 186: unique_ptr underlying_logger_;
: unique_ptr logger_;
Would be nice to briefly document these variables.
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@498
PS3, Line 498: if (FLAGS_data_cache_enable_tracing) {
: tracer_.reset(new Tracer(path_ + "/" + TRACE_FILE_NAME));
: RETURN_IF_ERROR(tracer_->Start());
: }
I wonder how this may affect the available storage space allocated for the data
cache ? The traces are relatively small compared to the cached data but I
wonder if a long running Impala process may accumulate a somewhat large trace
file.
On the other hand, tracing is mostly for experimentation so it's not a severe
concern for production environment.
http://gerrit.cloudera.org:8080/#/c/13425/3/be/src/runtime/io/data-cache.cc@887
PS3, Line 887: char buf[BUF_LEN];
nit: same name as 'buf' declared outside of the scope. While functionally
correct, it'd be less confusing to have no name collision.
--
To view, visit http://gerrit.cloudera.org:8080/13425
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2302c19abb5db19f1d3d1cd727a82977a9e2ba9c
Gerrit-Change-Number: 13425
Gerrit-PatchSet: 3
Gerrit-Owner: Todd Lipcon
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 03 Jun 2019 22:37:41 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 6: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 22:42:48 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Mon, 03 Jun 2019 22:45:06 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 4: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 22:47:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@117 PS4, Line 117: class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite): Could you add a TODO here references the JIRA you filed and mentioning that a lot of these tests are close copies of tests from query_test/test_kudu.py -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 23:02:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8603: remove confusing logging from custom cluster tests
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13502 ) Change subject: IMPALA-8603: remove confusing logging from custom cluster tests .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3489/ : 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/13502 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I96144fdbe6cc9bf0f01a9951420ee6f281fa6649 Gerrit-Change-Number: 13502 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Marshall Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Mon, 03 Jun 2019 23:04:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8536: Add Scalable Pool Configuration to Admission Controller.
Tim Armstrong has posted comments on this change. (
http://gerrit.cloudera.org:8080/13307 )
Change subject: IMPALA-8536: Add Scalable Pool Configuration to Admission
Controller.
..
Patch Set 6:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller-test.cc
File be/src/scheduling/admission-controller-test.cc:
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller-test.cc@71
PS6, Line 71: /// Build a QuerySchedule object.
Comment could be more descriptive.
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller-test.cc@74
PS6, Line 74: DCHECK(num_hosts > 0);
DCHECK_GT
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller-test.cc@175
PS6, Line 175: /// Check the calculations made by GetMaxQueuedForPool and
GetMaxRequestsForPool are
Extra spaces after ///
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller-test.cc@204
PS6, Line 204: /// Make an AdmissionController
Comment is a little uninformative. Maybe "Make an AdmissionController with some
dummy parameters" or something like that.
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller.h
File be/src/scheduling/admission-controller.h:
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller.h@583
PS6, Line 583: int64_t cluster_size_snapshot, bool admit_from_queue,
string* not_admitted_reason);
We generally use std::string in .h since the rule is that headers shouldn't
import things into the global namespace. Some headers we depend on don't follow
this rule, so std::string got into the global namespace somehow, but we
shouldn't depend on that.
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller.cc
File be/src/scheduling/admission-controller.cc:
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller.cc@1390
PS6, Line 1390: int64_t AdmissionController::GetClusterSize() {
I think we need to be a little careful with this being O(cluster size). If it
was used in the wrong context the runtime could end up blowing up. I think it's
ok with the current usage pattern, but it might be worth caching this value in
ClusterMembershipMgr::Snapshot to avoid repeated recomputations.
This is kind-of premature optimisation, but I think any perf issues might rear
their head in inconvenient situations, e.g. very high query concurrency.
http://gerrit.cloudera.org:8080/#/c/13307/6/be/src/scheduling/admission-controller.cc@1445
PS6, Line 1445: &&
Should this be ||? Since we shouldn't be able to submit queries to a pool if
either it has 0 request or 0 memory.
http://gerrit.cloudera.org:8080/#/c/13307/6/common/thrift/ImpalaInternalService.thrift
File common/thrift/ImpalaInternalService.thrift:
http://gerrit.cloudera.org:8080/#/c/13307/6/common/thrift/ImpalaInternalService.thrift@695
PS6, Line 695: :
inconsistent whitespace before colon
--
To view, visit http://gerrit.cloudera.org:8080/13307
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If47508728124076f3b9200c27cffc989f7a4f188
Gerrit-Change-Number: 13307
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Sherman
Gerrit-Reviewer: Andrew Sherman
Gerrit-Reviewer: Bikramjeet Vig
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Mon, 03 Jun 2019 23:24:13 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8143: Enhance DoRpcWithRetry().
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/12672 ) Change subject: IMPALA-8143: Enhance DoRpcWithRetry(). .. Patch Set 11: Code-Review+2 (6 comments) LGTM. Please address the nits. http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.h File be/src/rpc/impala-service-pool.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.h@72 PS11, Line 72: string char* See https://google.github.io/styleguide/cppguide.html#Static_and_Global_Variables http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.cc File be/src/rpc/impala-service-pool.cc: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/impala-service-pool.cc@192 PS11, Line 192: !debug_status.ok() UNLIKELY(!debug_status.ok()) http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.h File be/src/rpc/rpc-mgr-test.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.h@281 PS11, Line 281: getNumberOfCalls() nit: number_of_calls() or GetNumberOfCalls() http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.cc File be/src/rpc/rpc-mgr-test.cc: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.cc@310 PS11, Line 310: NULL nit: nullptr http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr-test.cc@331 PS11, Line 331: // DoRpcWithRetry will fail 1 in 2^40 times. : ASSERT_TRUE(status.ok()); : } : // There will be no overflows (i.e. service too busy) 1 in 2^40 times. : ASSERT_GT(overflow_metric->GetValue(), 0); :-). Subtle nits: Although they are similar this is referring to the probability of the debug action not kicking in at all in the loop above is: (1/2)^40 times. The other one on line 331 refers to the probability of the debugging action kicking in 40 times in a row. I suppose it's slightly clearer to state in (1/2)^num_rpc_retry_calls and (1/2)^num_retries. Please feel free to ignore as they are rather minor nits. http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr.h File be/src/rpc/rpc-mgr.h: http://gerrit.cloudera.org:8080/#/c/12672/11/be/src/rpc/rpc-mgr.h@183 PS11, Line 183: /// Pass 'debug_action' to DebugAction() to potentially inject errors. Please add a TODO: TODO: Clean up this interface. Replace the debug action with fault injection in RPC callbacks or other places. -- To view, visit http://gerrit.cloudera.org:8080/12672 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia9693151c35e02235665b3c285a48c585973d390 Gerrit-Change-Number: 12672 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Sherman Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 23:30:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13375 to look at the new patch set (#7). Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration This commit supports the actual handling of CREATE TABLE DDL for managed Kudu tables when integration with Hive Metastore is enabled. When Kudu/HMS integration is enabled, for CREATE TABLE statement, Impala can rely on Kudu to create the table in the HMS. Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 --- M fe/src/main/java/org/apache/impala/catalog/KuduTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java M fe/src/main/java/org/apache/impala/util/MetaStoreUtil.java M tests/common/custom_cluster_test_suite.py M tests/common/kudu_test_suite.py M tests/common/skip.py M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 10 files changed, 306 insertions(+), 20 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/75/13375/7 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13409 to look at the new patch set (#2). Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration This commit intends to support the actual handling of ALTER/RENAME TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. However, currently Kudu is considered as the source of truth of the table schema, so when ALTER TABLE (ADD/DROP COLUMN/RANGE_PARTITION), Impala always directly alters/loads the Kudu tables. Thus, this commit only updates RENAME TABLE DDL, so that after the table is renamed in the Kudu, relies on Kudu to rename the table in the HMS. Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java A testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test M tests/custom_cluster/test_kudu.py M tests/query_test/test_kudu.py 4 files changed, 666 insertions(+), 15 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13409/2 -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@117 PS4, Line 117: class TestKuduHMSIntegration(CustomClusterTestSuite, KuduTestSuite): > Could you add a TODO here references the JIRA you filed and mentioning that Done http://gerrit.cloudera.org:8080/#/c/13400/4/tests/custom_cluster/test_kudu.py@284 PS4, Line 284: ) > flake8: W292 no newline at end of file Done -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 23:32:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/13409 )
Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS
integration
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13409/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
File fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java:
http://gerrit.cloudera.org:8080/#/c/13409/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2584
PS1, Line 2584: if (isManagedKuduTable) {
: Preconditions.checkState(KuduTable.isKuduTable(m
> nit: mind updating the comment to explain the conditional block, instead of
Done
http://gerrit.cloudera.org:8080/#/c/13409/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2797
PS1, Line 2797: tbl.getMetaStoreTable().deepCopy();
> Could you clarify what we need to do here? How does this code need to be up
I think we should throw exception for alter kudu.table_name property for
managed tables as it is disallowed since IMPALA_5654. Filed a JIRA to track it.
http://gerrit.cloudera.org:8080/#/c/13409/1/testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_hms_alter.test:
PS1:
> Does the .test format allow for file-level comments? If so, might be worth
kudu_alter.test cannot be used because it contains tests with hardcoded legacy
table naming 'impala::' . I try to refactor it but it looks like not easy to.
--
To view, visit http://gerrit.cloudera.org:8080/13409
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269
Gerrit-Change-Number: 13409
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao
Gerrit-Reviewer: Alexey Serbin
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Grant Henke
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Marshall
Gerrit-Comment-Date: Mon, 03 Jun 2019 23:32:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Hello Thomas Marshall, Alexey Serbin, Andrew Wong, Grant Henke, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13400 to look at the new patch set (#5). Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration This commit supports the actual handling of DROP TABLE DDL for managed Kudu tables with Kudu's integration with the Hive Metastore. When Kudu/HMS integration is enabled, after the table is dropped in the Kudu, relies on Kudu to drop the table in the HMS. Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc --- M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M tests/custom_cluster/test_kudu.py 2 files changed, 148 insertions(+), 41 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/13400/5 -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 2: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4396/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Mon, 03 Jun 2019 23:33:21 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Patch Set 3: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Jun 2019 00:13:22 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Patch Set 3: Verified+1 Only commit message changed so I'll carry verification. -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Jun 2019 00:13:19 + Gerrit-HasComments: No
[Impala-ASF-CR] Disable custom cluster/service FE tests on S3
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13500 ) Change subject: Disable custom cluster/service FE tests on S3 .. Disable custom cluster/service FE tests on S3 This patch disables the custom cluster/service FE tests when running against S3, as only S3 FE tests should be ran under such environment. Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Reviewed-on: http://gerrit.cloudera.org:8080/13500 Tested-by: Tim Armstrong Reviewed-by: Tim Armstrong --- M bin/run-all-tests.sh 1 file changed, 8 insertions(+), 6 deletions(-) Approvals: Tim Armstrong: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13500 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Iaa95fb1cef6349afafe3fd3677df74f7fa225245 Gerrit-Change-Number: 13500 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3491/ : 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/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 00:16:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3490/ : 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/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 00:15:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7957: Fix slot equivalences may be enforced multiple times
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13051 ) Change subject: IMPALA-7957: Fix slot equivalences may be enforced multiple times .. Patch Set 5: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13051 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ida2d5d8149b217e18ebae61e136848162503653e Gerrit-Change-Number: 13051 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Jun 2019 00:20:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3492/ : 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/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 00:32:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges
Michael Ho has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13369 ) Change subject: IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges .. IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges As shown in IMPALA-8561, there are some paths in the code which create uncacheable ScanRanges. These uncacheable ScanRanges have mtime of -1. 'mtime' is used for differentiating versions of files with the same names. An mtime == -1 means the cache entry could potentially be from any versions of a file with the same name. This change skips lookup or insertion of ScanRange with negative mtime, file offset or buffer length. Testing done: Added targeted test cases in data-cache-test Change-Id: I2294833b075a2ddcae956d9fdb04f3e85adb0391 --- M be/src/runtime/io/data-cache-test.cc M be/src/runtime/io/data-cache.cc M be/src/runtime/io/data-cache.h 3 files changed, 39 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/69/13369/2 -- To view, visit http://gerrit.cloudera.org:8080/13369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I2294833b075a2ddcae956d9fdb04f3e85adb0391 Gerrit-Change-Number: 13369 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong
[Impala-ASF-CR] IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges
Michael Ho has posted comments on this change. (
http://gerrit.cloudera.org:8080/13369 )
Change subject: IMPALA-8562: Data cache should skip insertion of uncacheable
ScanRanges
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13369/1/be/src/runtime/io/data-cache-test.cc
File be/src/runtime/io/data-cache-test.cc:
http://gerrit.cloudera.org:8080/#/c/13369/1/be/src/runtime/io/data-cache-test.cc@267
PS1, Line 267: ASSERT_FALSE(cache.Store(FNAME, -1000, 0, test_buffer(),
TEMP_BUFFER_SIZE));
> Maybe test with a different bogus mtime, e.g. -1 just to demonstrate th
Done
http://gerrit.cloudera.org:8080/#/c/13369/1/be/src/runtime/io/data-cache.cc
File be/src/runtime/io/data-cache.cc:
http://gerrit.cloudera.org:8080/#/c/13369/1/be/src/runtime/io/data-cache.cc@287
PS1, Line 287: explicit CacheKey(const string& filename, int64_t mtime,
int64_t offset)
> Maybe add DCHECKs here to make sure that invalid values didn't slip through
Done
http://gerrit.cloudera.org:8080/#/c/13369/1/be/src/runtime/io/data-cache.cc@660
PS1, Line 660: int64_t bytes_to_read, uint8_t* buffer) {
> I think I prefer "uncachable" instead of "illegitimate". In some sense read
Done
--
To view, visit http://gerrit.cloudera.org:8080/13369
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2294833b075a2ddcae956d9fdb04f3e85adb0391
Gerrit-Change-Number: 13369
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Ho
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Lars Volker
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Sahil Takiar
Gerrit-Reviewer: Thomas Marshall
Gerrit-Reviewer: Tim Armstrong
Gerrit-Comment-Date: Tue, 04 Jun 2019 01:07:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges
Michael Ho has posted comments on this change. ( http://gerrit.cloudera.org:8080/13369 ) Change subject: IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges .. Patch Set 2: Code-Review+1 Carry Lars' +1 -- To view, visit http://gerrit.cloudera.org:8080/13369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2294833b075a2ddcae956d9fdb04f3e85adb0391 Gerrit-Change-Number: 13369 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Jun 2019 01:07:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13369 ) Change subject: IMPALA-8562: Data cache should skip insertion of uncacheable ScanRanges .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3493/ : 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/13369 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2294833b075a2ddcae956d9fdb04f3e85adb0391 Gerrit-Change-Number: 13369 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Ho Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Michael Ho Gerrit-Reviewer: Sahil Takiar Gerrit-Reviewer: Thomas Marshall Gerrit-Reviewer: Tim Armstrong Gerrit-Comment-Date: Tue, 04 Jun 2019 01:59:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13465 ) Change subject: IMPALA-8505: Disallow setting Kudu table id in ALTER TABLE .. Patch Set 2: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13465 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibc1f3f87054db5e28f72a5b54ea5b3c040c1bf22 Gerrit-Change-Number: 13465 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 03:52:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13375 ) Change subject: IMPALA-8504 (part 2): Support CREATE TABLE statement with Kudu/HMS integration .. Patch Set 6: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icffe412395f47f5e07d97bad457020770cfa7502 Gerrit-Change-Number: 13375 Gerrit-PatchSet: 6 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 04:05:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13400 ) Change subject: IMPALA-8507: Support DROP TABLE statement with Kudu/HMS integration .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4399/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/13400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6d3b93957cc66009ad7a67fc513be2068f156abc Gerrit-Change-Number: 13400 Gerrit-PatchSet: 5 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 04:50:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13409 ) Change subject: IMPALA-8506: Support RENAME TABLE statement with Kudu/HMS integration .. Patch Set 2: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4396/ -- To view, visit http://gerrit.cloudera.org:8080/13409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If7155e0b385b8ad81eda0a84277bc85171a88269 Gerrit-Change-Number: 13409 Gerrit-PatchSet: 2 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Alexey Serbin Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Thomas Marshall Gerrit-Comment-Date: Tue, 04 Jun 2019 05:15:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8578: part 2: move metrics code to .cc files
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13501 to look at the new patch set (#3). Change subject: IMPALA-8578: part 2: move metrics code to .cc files .. IMPALA-8578: part 2: move metrics code to .cc files This moves a lot of metric function definitions into .cc files, to reduce the size of compilation units and to reduce the frequency of recompilation required when making changes to metrics. This moves most of the large, non-perf-critical metric functions into .cc files. For template classes, this requires explicitly instantiating all combinations of template parameters that are used in impala, including in tests. Disable weak-template-vtables warning because of spurious warnings on template instantiations. See https://bugs.llvm.org/show_bug.cgi?id=18733 Change-Id: I78ad045ded6e6a7b7524711be9302c26115b97b9 --- M .clang-tidy M be/src/exprs/expr-test.cc M be/src/rpc/rpc-trace.cc M be/src/rpc/rpc-trace.h M be/src/rpc/thrift-server.cc M be/src/runtime/bufferpool/reservation-tracker-test.cc M be/src/runtime/coordinator.cc M be/src/runtime/io/hdfs-file-reader.cc M be/src/runtime/krpc-data-stream-mgr.cc M be/src/runtime/runtime-filter-bank.cc M be/src/scheduling/admission-controller-test.cc M be/src/scheduling/admission-controller.cc M be/src/scheduling/scheduler.cc M be/src/service/client-request-state.cc M be/src/service/client-request-state.h 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 be/src/statestore/statestore.cc M be/src/statestore/statestore.h M be/src/util/CMakeLists.txt M be/src/util/auth-util.cc A be/src/util/collection-metrics.cc M be/src/util/collection-metrics.h M be/src/util/default-path-handlers.cc A be/src/util/histogram-metric.cc M be/src/util/histogram-metric.h M be/src/util/metrics-test.cc M be/src/util/metrics.cc M be/src/util/metrics.h M be/src/util/webserver.cc M be/src/util/webserver.h 33 files changed, 620 insertions(+), 458 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/13501/3 -- To view, visit http://gerrit.cloudera.org:8080/13501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I78ad045ded6e6a7b7524711be9302c26115b97b9 Gerrit-Change-Number: 13501 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8578: part 2: move metrics code to .cc files
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13501 ) Change subject: IMPALA-8578: part 2: move metrics code to .cc files .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3494/ : 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/13501 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I78ad045ded6e6a7b7524711be9302c26115b97b9 Gerrit-Change-Number: 13501 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Armstrong Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 04 Jun 2019 06:06:08 + Gerrit-HasComments: No
