Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19838 )
Change subject: IMPALA-11534: Skip reloading file metadata for some ALTER_TABLE events ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/19838/7/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/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@376 PS7, Line 376: db nit: Just confused, what does "db" stands for here? I thought it's "database" at the first glance.. Will "key" or "k" looks better? http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3908 PS7, Line 3908: !isCustomTblPropsChanged(beforeTable, afterTable)) { For supportability, we'd better split these to seperate if-clause and add logs like "detected schema changed", "detected owner changed" when the conditions match. So in production we can reason about the behavior by logs. Also, can we move these new methods to MetastoreEvents.java? So the logs can be tagged with event ids. It seems they don't need any private fields of CatalogServiceCatalog. http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3926 PS7, Line 3926: !beforeCols.get(i).getType().equals(afterCols.get(i).getType())) Need logs for these as well. http://gerrit.cloudera.org:8080/#/c/19838/7/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@3939 PS7, Line 3939: if (!Objects.equals(configBefore, configAfter)) return true; Need logs for this as well. -- To view, visit http://gerrit.cloudera.org:8080/19838 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia66b96a7c4b7f50fbf46b2e02296cd29a47347b6 Gerrit-Change-Number: 19838 Gerrit-PatchSet: 7 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 06 Jun 2023 13:36:26 +0000 Gerrit-HasComments: Yes
