Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21175 )
Change subject: IMPALA-12829: Skip processing transaction events if the table is HMS sync disabled. ...................................................................... Patch Set 16: (6 comments) http://gerrit.cloudera.org:8080/#/c/21175/16/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/21175/16/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3141 PS16, Line 3141: recreated at This log is misleading for the second case in the if. http://gerrit.cloudera.org:8080/#/c/21175/16/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3146 PS16, Line 3146: ValidWriteIdList previousWriteIdList = hdfsTable.getValidWriteIds(); Can you add a comment that this block deals with Hive ACID tables? Probably not everyone is familiar with Hive ACID http://gerrit.cloudera.org:8080/#/c/21175/16/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3149 PS16, Line 3149: isTableNeedsRefresh nit: maybe just tableNeedsRefresh? http://gerrit.cloudera.org:8080/#/c/21175/16/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3154 PS16, Line 3154: isTableNeedsRefresh = true; we could break out of the loop http://gerrit.cloudera.org:8080/#/c/21175/16/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4555 PS16, Line 4555: tbl); nit: seems to fit to one line http://gerrit.cloudera.org:8080/#/c/21175/16/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/21175/16/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@7472 PS16, Line 7472: addWriteIds Doesn't loadTableMetadata() reload the table anyway and fetches a fresh write id list? If not, then I am worried about breaking transaction semantics, as this write may be visible before another write that finished earlier. -- To view, visit http://gerrit.cloudera.org:8080/21175 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5d0ecb3b756755bc04c66a538a9ae6b88011a019 Gerrit-Change-Number: 21175 Gerrit-PatchSet: 16 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Thu, 05 Jun 2025 12:02:50 +0000 Gerrit-HasComments: Yes
