Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/23487 )
Change subject: IMPALA-14131: Add flag to configure the default value of 'impala.disableHmsSync' ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/23487/2/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/23487/2/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java@4906 PS2, Line 4906: LOG.debug("Table level for table {} or Db level for db {}, disable hms sync flag " + : "is not set. Global level flag is set to {} ", tbl.getTableName(), : tbl.getDbName(), globalDisableHmsSync); nit: This log is redundant with the caller sides, e.g. https://github.com/apache/impala/blob/ebbc67cf40bd856253d07c649028888d85c772cc/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java#L4831-L4833 Can we merge them? http://gerrit.cloudera.org:8080/#/c/23487/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java: http://gerrit.cloudera.org:8080/#/c/23487/2/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1263 PS2, Line 1263: debugLog("Table level for table {} or Db level for db {}, disable hms sync flag " + : "is not set. Global level flag is set to {} ", msTbl_.getTableName(), : dbName_, globalDisableHmsSync); Can we skip this log if globalDisableHmsSync is false? Otherwise, lots of events will show this log and it's useless when globalDisableHmsSync is false. http://gerrit.cloudera.org:8080/#/c/23487/2/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/23487/2/tests/custom_cluster/test_events_custom_configs.py@1847 PS2, Line 1847: self.run_stmt_in_hive( : """alter table {tb1} set tblproperties ('k'='v'); : alter table {tb2} set tblproperties ('k'='v');""" : .format(tb1=tbl1, tb2=tbl2)) Should we run these after the IM commands and verify their results? -- To view, visit http://gerrit.cloudera.org:8080/23487 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4ee617aed48575502d9cf5cf2cbea6ec897d6839 Gerrit-Change-Number: 23487 Gerrit-PatchSet: 2 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Thu, 16 Oct 2025 09:43:09 +0000 Gerrit-HasComments: Yes
