[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 )
Change subject: IMPALA-12832: Implicit invalidate metadata on event failures ...................................................................... Patch Set 21: (7 comments) http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc@166 PS16, Line 166: DEFINE_bool(invalidate_metadata_on_event_processing_failure, true, > naming: Have changed it to invalidate_metadata_on_event_processing_failure http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc@166 PS16, Line 166: DEFINE_bool(invalidate_metadata_on_event_processing_failure, true, : "This configuration is used to invalidate metadata for table(s) upon event process " : "failure other than HMS connection issues. The default value is true. When enabled, " : "invalidate metadata is performed automatically upon event process failure. " : "Otherwise, failure can put metastore event processor in non-active state."); : : DEFINE_bool(invalidate_global_metadata_on_event_pro > naming: Have changed it to invalidate_global_metadata_on_event_processing_failure http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc@185 PS16, Line 185: > The name suggests to me that bigger value means more failure. Correct. And the default value is 1.0 in the latest patch http://gerrit.cloudera.org:8080/#/c/21065/16/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/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1288 PS16, Line 1288: } > add " during event processing"? Done http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java File fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java: http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1007 PS16, Line 1007: catalog_.reset(); > Besides logging it would be great to have a metric for the number of table Shall take it up in another jira. http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1187 PS16, Line 1187: > e/ex could have more descriptive names, e.g. processingEx and onFailureEx Done http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3736 PS16, Line 3736: assertEquals(EventProcessorStatus.ACTIVE, eventsProcessor_.getStatus()); : } finally { > I think that the catch block could be omitted if we just rethrow the except Done -- To view, visit http://gerrit.cloudera.org:8080/21065 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia67fc04c995802d3b6b56f79564bf0954b012c6c Gerrit-Change-Number: 21065 Gerrit-PatchSet: 21 Gerrit-Owner: Anonymous Coward <[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: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Thu, 07 Mar 2024 03:11:17 +0000 Gerrit-HasComments: Yes
