Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/10543 )
Change subject: IMPALA-6119: Fix issue with multiple partitions sharing same location ...................................................................... Patch Set 2: (12 comments) http://gerrit.cloudera.org:8080/#/c/10543/1/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/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@189 PS1, Line 189: ). > nit:Java style Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@189 PS1, Line 189: from HdfsPar > nit: remove, kinda obvious Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1105 PS1, Line 1105: tition); > can be inferred from the partition object? Good point. Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1204 PS1, Line 1204: .s > nit:Java Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1205 PS1, Line 1205: updates l > updates Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1207 PS1, Line 1207: updat > nit: update? Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1214 PS1, Line 1214: Adds 'partition' to locationToPartMap_. > May be just say "Adds 'partition' to locationTo..." Rest is implicit. Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1216 PS1, Line 1216: addToLocationPartMap(Hd > call it addToLocationPartMap may be? Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@1468 PS1, Line 1468: : * Given a set of partition names, returns the correspon > I think the earlier description is more clear. I wanted to somehow include to the comment that we now search for all the HdfsPartitions that point where the ones received by parameter. Rephrased the comment. What do you think? http://gerrit.cloudera.org:8080/#/c/10543/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/10543/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2434 PS1, Line 2434: > nit: call it updatePartitionLocation()? Done http://gerrit.cloudera.org:8080/#/c/10543/1/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2435 PS1, Line 2435: applyAlterPartition(tbl, partition); : } finally { : > I don't think non HdfsTable is possible here. Look at catalog_.getHdfsParti Done http://gerrit.cloudera.org:8080/#/c/10543/1/tests/metadata/test_partition_metadata.py File tests/metadata/test_partition_metadata.py: http://gerrit.cloudera.org:8080/#/c/10543/1/tests/metadata/test_partition_metadata.py@159 PS1, Line 159: assert data.split('\t') == ['21', '6'] > Could you drop one of the partitions that point to the same partition dir a Nice catch as dropping a partition in this case would cause some issue. Note, that this is present without my code as well. Basically, when I drop a partition than the hdfs directory is deleted no matter if other partitions would use it. However, as far as I see Impala's metadata is not aware of this and treat the other partitions pointing to this location valid including their stored file descriptors. Once some data is queried from these partitions than a "No such file or directory" error is shown. An invalidate metadata then solves the issue with querying the table. Is that expected to keep the other partitions to the same location alive? I tested on 5.15 nightly and this issue is present there as well. -- To view, visit http://gerrit.cloudera.org:8080/10543 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2a54bc8224bcefe65b83de2df58bb84629f2aa4a Gerrit-Change-Number: 10543 Gerrit-PatchSet: 2 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 31 May 2018 16:30:16 +0000 Gerrit-HasComments: Yes
