Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/21065 )
Change subject: IMPALA-12832: Implicit invalidate metadata on event failures ...................................................................... Patch Set 16: (9 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(process_event_failure, true, naming: this include event processing somehow, e.g. invalidate_table_metadata_on_event_processing_error http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc@166 PS16, Line 166: DEFINE_bool(process_event_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(auto_global_invalidate_metadata, false, naming: this include event processing somehow, e.g. invalidate_global_metadata_on_event_processing_error http://gerrit.cloudera.org:8080/#/c/21065/16/be/src/catalog/catalog-server.cc@185 PS16, Line 185: inject_process_event_failure_ratio The name suggests to me that bigger value means more failure. 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: warnLog("Invalidate table {}.{} due to exception", dbName_, tblName_, e); add " during event processing"? 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: LOG.warn("Triggering auto global invalidation"); Besides logging it would be great to have a metric for the number of table level and global auto invalidations. http://gerrit.cloudera.org:8080/#/c/21065/16/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java@1187 PS16, Line 1187: e e/ex could have more descriptive names, e.g. processingEx and onFailureEx 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: } catch(Exception ex) { : throw ex; I think that the catch block could be omitted if we just rethrow the exception. http://gerrit.cloudera.org:8080/#/c/21065/16/tests/metadata/__init__.py File tests/metadata/__init__.py: http://gerrit.cloudera.org:8080/#/c/21065/16/tests/metadata/__init__.py@1 PS16, Line 1: # This file is needed to make the files in this directory a python module Is this new file needed? http://gerrit.cloudera.org:8080/#/c/21065/16/tests/metadata/test_event_processing_error.py File tests/metadata/test_event_processing_error.py: http://gerrit.cloudera.org:8080/#/c/21065/16/tests/metadata/test_event_processing_error.py@59 PS16, Line 59: assert EventProcessorUtils.get_event_processor_status() == "ACTIVE" All tests run with automatic invalidation, so get_event_processor_status() always returns "ACTIVE". Can you add a simple test checks if the error injection runs correctly, so that get_event_processor_status() doesn't return "ACTIVE"? -- 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: 16 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: Wed, 06 Mar 2024 18:25:56 +0000 Gerrit-HasComments: Yes
