Yu-Wen Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17697 )
Change subject: IMPALA-10801: Check the latest compaction Id before serving ACID table ...................................................................... Patch Set 14: (3 comments) > Patch Set 14: > > (2 comments) http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java File fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java: http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2194 PS14, Line 2194: AcidUtils.compare((HdfsTable) existingTbl, : updatedTbl.getValidWriteIds(), tableId) : >= 0) > Is it guaranteed that the existingTbl will have compacted files if it has a No, it is not guaranteed. In this case, the table is coming from a full table loading and I supposed at this time the client who sent the query already acquired read/write lock on the table. Therefore, the file metadata loaded won't be cleaned for the lifetime of the query. We don't need to worry other queries because the file metadata will be updated next time when there is any compaction. If I understand the original design correctly, the conditions here make sure we only update once for many requests to a single table. After my change, refreshing file metadata also changes catalogVersion. Let's say there are one table loading and one file metadata refreshing happening together. If file metadata refreshing is finished earlier and catalogVersion is updated, we should not discard the result of full table loading here since we need to return the table with most recent validWriteIdList. http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/main/java/org/apache/impala/catalog/CompactionInfoLoader.java File fe/src/main/java/org/apache/impala/catalog/CompactionInfoLoader.java: http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/main/java/org/apache/impala/catalog/CompactionInfoLoader.java@38 PS14, Line 38: CatalogServiceCatalog catalog, GetLatestCommittedCompactionInfoRequest request) > nit: Only pass MetastoreClient instead of whole catalog Object ? Passing catalog here so we can get a client from pool only when needed. http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java File fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java: http://gerrit.cloudera.org:8080/#/c/17697/14/fe/src/test/java/org/apache/impala/catalog/PartialCatalogInfoWriteIdTest.java@564 PS14, Line 564: Assert.assertEquals > A comment above this line saying that compacted tables/partitions should on Not sure about ORC tables. Will check. -- To view, visit http://gerrit.cloudera.org:8080/17697 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I86a112a77980fef7f6238978bc9668a65262101e Gerrit-Change-Number: 17697 Gerrit-PatchSet: 14 Gerrit-Owner: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Sourabh Goyal <soura...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Yu-Wen Lai <yu-wen....@cloudera.com> Gerrit-Comment-Date: Thu, 05 Aug 2021 21:39:48 +0000 Gerrit-HasComments: Yes
