You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org> on 2018/07/06 16:19:48 UTC
[Impala-ASF-CR] IMPALA-6994:Avoid reloading a table's HMS data for file-only operations
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/10587 )
Change subject: IMPALA-6994:Avoid reloading a table's HMS data for file-only operations
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-6994:Avoid reloading a table's HMS data for file-only operations
nit: add a space
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@9
PS2, Line 9:
remove the description block's indent
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@10
PS2, Line 10: unecessary
nit: check spelling and remove extra spaces.
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@9
PS2, Line 9: unpartitioned HDFS table,
: or to an existing partition
simplify to just HDFS table, or is there some case that's being excluded?
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@14
PS2, Line 14: This extra call to HMS introduces a point of failure, we need to handle
: for error scenario when Hive MetaStore crashes
So is this change an optimization, a correctness/robustness fix, or both? Also, I don't understand the point about a failure-- pls add an example with a sequence of steps that currently leads to an error. Can add such an example to the jira.
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@15
PS2, Line 15: This change removes the
: extra call to Hive Meta Store for the case when a row is inserted to an
: existing partition in HDFS table or when a row is added to unpartitioned
: table. Thus, an optimization as it reduces the call to Hive Meta Store
: during Update of Catalog.
lots of redundancy here with the prev. paragraph-- pls condense or remove.
http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@21
PS2, Line 21: Testing: Ran core test without failure.
Is there any new scenario that you want to test for this change? If existing tests cover it, is there any way to assert that this change is operating as expected?
http://gerrit.cloudera.org:8080/#/c/10587/2/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/10587/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1252
PS2, Line 1252: nonewPartitionHint
needs a comment.
--
To view, visit http://gerrit.cloudera.org:8080/10587
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I331bb0371fde287f43a85b025b4f98cb45f3eb3c
Gerrit-Change-Number: 10587
Gerrit-PatchSet: 2
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 16:19:48 +0000
Gerrit-HasComments: Yes