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 10: (6 comments) http://gerrit.cloudera.org:8080/#/c/17697/10/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/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@2127 PS10, Line 2127: Get non-ACID table with writeIdList: > This text here comes as a message for the IllegalStateException thrown and Ack http://gerrit.cloudera.org:8080/#/c/17697/10/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3479 PS10, Line 3479: If there is an ongoing loading task, we don't reload file metadata but wait for the : * loading task completed and return the table just loaded. > this comment is stale now that removed the loadReq logic. Ack http://gerrit.cloudera.org:8080/#/c/17697/10/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/17697/10/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java@41 PS10, Line 41: Log > is this import needed? Ack http://gerrit.cloudera.org:8080/#/c/17697/10/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java File fe/src/main/java/org/apache/impala/catalog/IcebergTable.java: http://gerrit.cloudera.org:8080/#/c/17697/10/fe/src/main/java/org/apache/impala/catalog/IcebergTable.java@345 PS10, Line 345: hdfsTable_.writeLock().lock(); > why is this needed? The table locks are generally taken at CatalogOpExecuto This load fails after adding a precondition check for locking in HdfsTable.loadFileMetadataForPartitions. I suppose the lock is not taken at CatalogOpExecutor because hdfsTable_ is the internal object of icebergTable. http://gerrit.cloudera.org:8080/#/c/17697/10/fe/src/main/java/org/apache/impala/catalog/TableLoader.java File fe/src/main/java/org/apache/impala/catalog/TableLoader.java: http://gerrit.cloudera.org:8080/#/c/17697/10/fe/src/main/java/org/apache/impala/catalog/TableLoader.java@115 PS10, Line 115: able.writeLock().lock(); > Why is this needed? Same as icebergTable. After adding a precondition check for locking in HdfsTable.loadFileMetadataForPartitions, this function would fail without taking lock. http://gerrit.cloudera.org:8080/#/c/17697/10/testdata/cluster/ranger/setup/policy_5_revised.json File testdata/cluster/ranger/setup/policy_5_revised.json: http://gerrit.cloudera.org:8080/#/c/17697/10/testdata/cluster/ranger/setup/policy_5_revised.json@8 PS10, Line 8: 5 > how is this change related? If it is not can we remove it from this patch a Since this patch bumps up cdp version and the new version of ranger would cause failure of create-load-data.sh. If I don't put this here, I cannot get a green testing. Or should I create a seperate commit to bump up cdp version? -- 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: 10 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: Wed, 28 Jul 2021 21:40:09 +0000 Gerrit-HasComments: Yes
