[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Reviewed-on: http://gerrit.cloudera.org:8080/13215 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 348 insertions(+), 3 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 19 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 18: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 18 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 16 May 2019 21:12:15 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 18: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4271/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 18 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 16 May 2019 15:51:19 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 18: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 18 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 16 May 2019 15:51:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 17:
> Patch Set 17:
>
> > Patch Set 17: Verified-1
> >
> > Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4269/
>
> The two failures are not related:
> https://jenkins.impala.io/job/ubuntu-16.04-dockerised-tests/261/
> The two test failures have the similar error message:
> Exception: ("DB {0} didn't show up after {1}s", 'hms_sanity_db', 30)
> Exception: ("DB {0} didn't show up after {1}s", 'hive_test_desc_db', 30)
> Looks like the testing cluster has some performance issue
The failures are exact the same as the tests failures for a different
patch(which only has tests changes):
https://gerrit.cloudera.org/c/13339
https://jenkins.impala.io/job/ubuntu-16.04-dockerised-tests/258/
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 17
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 16 May 2019 04:27:12 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 17:
> Patch Set 17: Verified-1
>
> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4269/
The two failures are not related:
https://jenkins.impala.io/job/ubuntu-16.04-dockerised-tests/261/
The two test failures have the similar error message:
Exception: ("DB {0} didn't show up after {1}s", 'hms_sanity_db', 30)
Exception: ("DB {0} didn't show up after {1}s", 'hive_test_desc_db', 30)
Looks like the testing cluster has some performance issue
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 17
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 16 May 2019 04:13:01 +
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 17: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4269/ -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 17 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 16 May 2019 03:42:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 17: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 17 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 22:22:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 16: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 16 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 22:22:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 17: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4269/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 17 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 22:22:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 16: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3243/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 16 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 22:15:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 16: submit patch-16, run core tests, no faiures. https://master-02.jenkins.cloudera.com/job/impala-private-parameterized/5051/ -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 16 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 21:42:42 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#16). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 348 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/16 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 16 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 15: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4262/ -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 15 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 11:52:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 15 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 06:29:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4262/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 15 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 06:29:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 14: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 14 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Wed, 15 May 2019 06:28:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3225/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 14 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 May 2019 22:20:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#14). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 346 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/14 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 14 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3213/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 11 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 May 2019 10:52:47 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#11). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 346 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/11 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 11 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3212/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 10 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 May 2019 05:42:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#10). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 344 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/10 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 10 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3211/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 9 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 May 2019 04:14:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 8:
(13 comments)
Will submit patch 9 for the issues.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java@255
PS8, Line 255: return -1L;
> agree with Sudanshu's earlier comments -- I think it's better to throw Unsu
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java@2
PS8, Line 2: // // or more contributor license agreements. See the NOTICE file
> nit: double // here
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java@18
PS8, Line 18:
> Doesn't this need to be in the same package as the Hive ValidWriteIdClass s
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@387
PS8, Line 387: public static String fetchValidWriteIdsString(IMetaStoreClient
client,
> why do we need this class if we already have the above one, and the shim cl
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@399
PS8, Line 399: if (validWriteIds != null) {
> Why check against null here instead of Preconditions.checkNotNull()?
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@412
PS8, Line 412: return partition == null ? -1L : partition.getWriteId();
> Same -- we shouldn't be checking for null and papering over it. It should b
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@238
PS8, Line 238: System.lineSeparator
> nit: we just use \n elsewhere
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@243
PS8, Line 243: validIdsBuf.append(" ");
> why this extra whitespace?
To indent. If not, the output looks ugly.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java
File fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java@172
PS8, Line 172:* @return the writeId stored in hms for the partition
> Per comments elsewhere, I think we should either change these to Long (to a
Add common.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@618
PS8, Line 618: //-1 means writeId_ is irrelevant(not supported).
> nit: space after //
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1022
PS8, Line 1022:
: if (thriftPartition.isSetWrite_id()) {
: partition.writeId_ = thriftPartition.getWrite_id();
: } else {
: partition.writeId_ = -1l;
: }
> Use a ternary?
Done
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@924
PS8, Line 924: //TODO writeIDs may also be loaded in other code
paths.
> I'm still not quite sure I understand this TODO
Will remove it.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
File fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java@235
PS8, Line 235: te
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#9). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift A fe/src/compat-hive-2/java/org/apache/hadoop/hive/common/ValidWriteIdList.java M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 343 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/9 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 9 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/13215/9/fe/src/main/java/org/apache/impala/catalog/Table.java File fe/src/main/java/org/apache/impala/catalog/Table.java: http://gerrit.cloudera.org:8080/#/c/13215/9/fe/src/main/java/org/apache/impala/catalog/Table.java@287 PS9, Line 287: ValidWriteIdList validWriteIds = MetastoreShim.fetchValidWriteIds(client, tblFullName); line too long (93 > 90) -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 9 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 14 May 2019 03:47:12 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 8:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java@255
PS8, Line 255: return -1L;
agree with Sudanshu's earlier comments -- I think it's better to throw
UnsupportedOperationException here, or to make these functions return
Optional (or just Long with null indicating unset) so that we don't
accidentally end up treating -1 as a valid write ID anywhere.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java@2
PS8, Line 2: // // or more contributor license agreements. See the NOTICE file
nit: double // here
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java@18
PS8, Line 18:
Doesn't this need to be in the same package as the Hive ValidWriteIdClass so
that the shim methods return the same type?
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@387
PS8, Line 387: public static String fetchValidWriteIdsString(IMetaStoreClient
client,
why do we need this class if we already have the above one, and the shim class
for ValidWriteIds?
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@399
PS8, Line 399: if (validWriteIds != null) {
Why check against null here instead of Preconditions.checkNotNull()?
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@412
PS8, Line 412: return partition == null ? -1L : partition.getWriteId();
Same -- we shouldn't be checking for null and papering over it. It should be an
error to pass null to these functions.
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@238
PS8, Line 238: System.lineSeparator
nit: we just use \n elsewhere
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@243
PS8, Line 243: validIdsBuf.append(" ");
why this extra whitespace?
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java
File fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java@172
PS8, Line 172:* @return the writeId stored in hms for the partition
Per comments elsewhere, I think we should either change these to Long (to allow
for null when we have no write id known) or at least clearly document than '-1
indicates unknown' so that callers know they need to handle that. (I see you
documented that on the member variable in one spot, but I think it's also
important to document on these cross-system interfaces)
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@618
PS8, Line 618: //-1 means writeId_ is irrelevant(not supported).
nit: space after //
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1022
PS8, Line 1022:
: if (thriftPartition.isSetWrite_id()) {
: partition.writeId_ = thriftPartition.getWrite_id();
: } else {
: partition.writeId_ = -1l;
: }
Use a ternary?
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/8/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@924
PS8, Line 924: //TODO writeIDs
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3154/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 8 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Thu, 09 May 2019 20:48:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 7:
(4 comments)
Push patch 8
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS7, Line 64: Hive 2.
> Hive 3?
Done
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@390
PS7, Line 390: client.getValidWriteIds(tableFullName);
> nit: could be used to initialize 'writeIds' in previous line.
Done
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@395
PS7, Line 395: sting
> nit: string
Done
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@401
PS7, Line 401: {
> nit: inconsistent formatting, please put it to previous line
Done
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 09 May 2019 18:54:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#8). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java A fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 357 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/8 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 8 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Zoltan Borok-Nagy has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@479
PS6, Line 479: iff
> It means if and only if. It will not set the list when it is null, so it is
Sorry, I don't see that very often. Or I always thought that it is a typo
whenever I saw it :)
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@64
PS7, Line 64: Hive 2.
Hive 3?
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@390
PS7, Line 390: client.getValidWriteIds(tableFullName);
nit: could be used to initialize 'writeIds' in previous line.
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@395
PS7, Line 395: sting
nit: string
http://gerrit.cloudera.org:8080/#/c/13215/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@401
PS7, Line 401: {
nit: inconsistent formatting, please put it to previous line
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 7
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Reviewer: Zoltan Borok-Nagy
Gerrit-Comment-Date: Thu, 09 May 2019 17:02:56 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3103/ : 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/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 7 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 07 May 2019 18:45:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Zoltan Borok-Nagy, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#7). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java A fe/src/compat-hive-2/java/org/apache/impala/compat/ValidWriteIdList.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 15 files changed, 358 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/7 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 7 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Yongzhi Chen has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 6:
(14 comments)
submit patch 7
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@479
PS6, Line 479: iff
> nit: typo
It means if and only if. It will not set the list when it is null, so it is
right spell.
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@482
PS6, Line 482: :
> nit: maybe you could explain how can we decode these parts (if they are not
I will add a method to parse the string to ValidWriteIdList
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@482
PS6, Line 482: //
> Are we guaranteed that Hive team will keep this string backward compatible?
We use their function do the converts (writeToString, fromTheString). Hive uses
in their clients too.
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@374
PS6, Line 374: * @return the list of valid write IDs for the table in a string
> Nit: or null if there are no validWriteIds
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@388
PS6, Line 388: public static long getHighestWriteId(String validWriteIds)
> This method is no longer used. Let's remove it until we have a use
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@399
PS6, Line 399: which is missing
:* in HMS 2
> nit: maybe instead of writing that it is missing in HMS 2, you could briefl
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@407
PS6, Line 407: which is missing
:* in HMS 2
> nit: same as above
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@236
PS6, Line 236: StringBuilder validIdsBuf = new StringBuilder("Loaded
ValidWriteIdLists: ");
> For my understanding, how do we use timeline?
It is a little odd to have time at the end. Use timeline to put the writeID
info into profile. we may improve timeline later or do you have better
suggestions?
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
File fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@111
PS6, Line 111: public long getWriteId() {
> maybe we can implement these as default methods in the FeTable interface?
This DataSourceTable class extends Table class and implements
FeDataSourceTable. Put a default implementation for FeTable will make it more
complicated. And because Table already implements these methods,
DataSourceTable has to override here(I assume DataSourceTable does not support
writeId related operations).
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@649
PS6, Line 649: writeId_ = msPartition != null ?
> Nit: Handle null case in shim so that every call does not have to handle it
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1026
PS6, Line 1026: }
> Nit: Use ternary or put else in the above line
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@924
PS6, Line 924: //TODO put the writeID load here for now.
> Can you rephrase this to explain the _future_ state, not the current state?
Done
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/Table.java@287
PS6, L
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/13215 ) Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. Patch Set 6: (4 comments) Just did a very quick pass, will look at it again tomorrow http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift File common/thrift/CatalogObjects.thrift: http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@479 PS6, Line 479: iff nit: typo http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@482 PS6, Line 482: : nit: maybe you could explain how can we decode these parts (if they are not human-readable), or at least give some pointers. http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@399 PS6, Line 399: which is missing :* in HMS 2 nit: maybe instead of writing that it is missing in HMS 2, you could briefly explain what is a write id. http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@407 PS6, Line 407: which is missing :* in HMS 2 nit: same as above -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen Gerrit-Reviewer: Zoltan Borok-Nagy Gerrit-Comment-Date: Tue, 07 May 2019 16:14:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@388
PS6, Line 388: public static long getHighestWriteId(String validWriteIds)
This method is no longer used. Let's remove it until we have a use
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java
File fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java@111
PS6, Line 111: public long getWriteId() {
maybe we can implement these as default methods in the FeTable interface?
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java
File fe/src/main/java/org/apache/impala/catalog/HdfsTable.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@924
PS6, Line 924: //TODO put the writeID load here for now.
Can you rephrase this to explain the _future_ state, not the current state?
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/Table.java
File fe/src/main/java/org/apache/impala/catalog/Table.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/Table.java@287
PS6, Line 287: LOG.warn("Could not get valid writeIds for: " +
tblFullName, e);
shouldn't we be passing this through as a tableloadingexception of some kind?
it seems bad to silently drop the information.
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java
File fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java@99
PS6, Line 99: <=
MetastoreShim.getHighestWriteId(t.getValidWriteIds()));
Instead can we assert on t.getValidWriteIds().isWriteIdValid(t.getWriteId())?
(we may need to add a shim class for ValidWriteIdList btw -- ie make a stub
implementation in the hive-2-shims source directly that has the same method
names, but throws UnsupportedOperationException)
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Comment-Date: Tue, 07 May 2019 08:12:04 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Sudhanshu Arora has posted comments on this change. (
http://gerrit.cloudera.org:8080/13215 )
Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and
partition
..
Patch Set 6:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift
File common/thrift/CatalogObjects.thrift:
http://gerrit.cloudera.org:8080/#/c/13215/6/common/thrift/CatalogObjects.thrift@482
PS6, Line 482: //
Are we guaranteed that Hive team will keep this string backward compatible?
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java@231
PS6, Line 231: return null;
throw UnsupportedOperationException.
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java
File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@374
PS6, Line 374: * @return the list of valid write IDs for the table in a string
Nit: or null if there are no validWriteIds
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java
File fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java@236
PS6, Line 236: StringBuilder validIdsBuf = new StringBuilder("Loaded
ValidWriteIdLists: ");
For my understanding, how do we use timeline?
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java
File fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java:
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@649
PS6, Line 649: writeId_ = msPartition != null ?
Nit: Handle null case in shim so that every call does not have to handle it.
http://gerrit.cloudera.org:8080/#/c/13215/6/fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java@1026
PS6, Line 1026: }
Nit: Use ternary or put else in the above line
--
To view, visit http://gerrit.cloudera.org:8080/13215
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54
Gerrit-Change-Number: 13215
Gerrit-PatchSet: 6
Gerrit-Owner: Yongzhi Chen
Gerrit-Reviewer: Csaba Ringhofer
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Sudhanshu Arora
Gerrit-Reviewer: Todd Lipcon
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Reviewer: Yongzhi Chen
Gerrit-Comment-Date: Mon, 06 May 2019 23:48:48 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8438: Store WriteId and ValidWriteId list for table and partition
Hello Vihang Karajgaonkar, Sudhanshu Arora, Todd Lipcon, Csaba Ringhofer, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/13215 to look at the new patch set (#6). Change subject: IMPALA-8438: Store WriteId and ValidWriteId list for table and partition .. IMPALA-8438: Store WriteId and ValidWriteId list for table and partition This happens when tables load metadata from HMS. Add MetastoreShim functions to support HMS3 only functions. Add validwriteIdlists to query profile through timeline. Tests: Manually tests HMS2 and HMS3, using log files to check Unit tests against HMS3 ToDo: WriteId and valid writeIds can be fetched in other time, need more study on that. Profile example: Query Compilation: 5s057ms - Metadata load started: 63.006ms (63.006ms) - Metadata load finished. loaded-tables=2/2...: 4s801ms (4s738ms) - Loaded ValidWriteIdLists: acid.insert_only_no_partitions:6:9223372036854775807:: acid.insert_only_with_partitions:3:9223372036854775807:: : 4s921ms (120.580ms) - Analysis finished: 4s929ms (8.013ms) Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 --- M common/thrift/CatalogObjects.thrift M fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java M fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java M fe/src/main/java/org/apache/impala/analysis/StmtMetadataLoader.java M fe/src/main/java/org/apache/impala/catalog/DataSourceTable.java M fe/src/main/java/org/apache/impala/catalog/FeCatalogUtils.java M fe/src/main/java/org/apache/impala/catalog/FeFsPartition.java M fe/src/main/java/org/apache/impala/catalog/FeTable.java M fe/src/main/java/org/apache/impala/catalog/HdfsPartition.java M fe/src/main/java/org/apache/impala/catalog/HdfsTable.java M fe/src/main/java/org/apache/impala/catalog/Table.java M fe/src/main/java/org/apache/impala/catalog/local/LocalFsPartition.java M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java M fe/src/test/java/org/apache/impala/analysis/StmtMetadataLoaderTest.java 14 files changed, 262 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/13215/6 -- To view, visit http://gerrit.cloudera.org:8080/13215 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6edbd64424edf0ba88af110ab8b958a1966b8b54 Gerrit-Change-Number: 13215 Gerrit-PatchSet: 6 Gerrit-Owner: Yongzhi Chen Gerrit-Reviewer: Csaba Ringhofer Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Sudhanshu Arora Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Reviewer: Yongzhi Chen
