Pranay Singh 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 Done http://gerrit.cloudera.org:8080/#/c/10587/2//COMMIT_MSG@10 PS2, Line 10: unecessary > nit: check spelling and remove extra spaces. Done 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? Done 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 existin Yes I want to test the scenario where concurrent removal of partition and insertion of row to the same partition happens. a) Impala is inserting the row to an existing partition b) Hive is removing the partition at the same time. 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. Done 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. Added the comment http://gerrit.cloudera.org:8080/#/c/10587/2/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1365 PS2, Line 1365: loadMetadataAndDiskIds(partitionsToUpdateFileMdByPath, true); > any way to refactor this function so you don't need to duplicate this code Refactored the code and added a new function to reload the partition. http://gerrit.cloudera.org:8080/#/c/10587/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/10587/2/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@640 PS2, Line 640: private void loadTableMetadata(Table tbl, long newCatalogVersion, > the arguments here are getting quite unwieldly, and it seems that the two e Made changes to the code to use enum MetadataLoadFlag -- 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 <[email protected]> Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Reviewer: Vuk Ercegovac <[email protected]> Gerrit-Comment-Date: Thu, 19 Jul 2018 18:27:08 +0000 Gerrit-HasComments: Yes
