Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/21031 )
Change subject: IMPALA-12709: Add support for hierarchical metastore event processing ...................................................................... Patch Set 26: (9 comments) http://gerrit.cloudera.org:8080/#/c/21031/26//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/21031/26//COMMIT_MSG@30 PS26, Line 30: only after the alter database event is processed. Consider the scenario: a database event, and then there are a bunch of table events, and then the db is renamed to new db. What happens at the event processor currently and with the hierarchical event processor when it is processing table events and HMS has new db in it? http://gerrit.cloudera.org:8080/#/c/21031/26/be/src/catalog/catalog-server.cc File be/src/catalog/catalog-server.cc: http://gerrit.cloudera.org:8080/#/c/21031/26/be/src/catalog/catalog-server.cc@217 PS26, Line 217: DEFINE_int32(hms_event_polling_interval_ms, 0, Can you add some description about why we need a ms interval when we already have a secs interval? http://gerrit.cloudera.org:8080/#/c/21031/26/be/src/common/global-flags.cc File be/src/common/global-flags.cc: http://gerrit.cloudera.org:8080/#/c/21031/26/be/src/common/global-flags.cc@294 PS26, Line 294: "hms_event_polling_interval_ms is used to specify the polling interval in " Should we say here that 'hms_event_polling_interval_ms' is used by hierarchical_event_processer? http://gerrit.cloudera.org:8080/#/c/21031/26/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/21031/26/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@637 PS26, Line 637: processEventProcessorDebugAction(ddlRequest.getQuery_options().getDebug_action()); Do we need a debug action when we have this: https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java#L1415-L1417? Also, which test is using this? http://gerrit.cloudera.org:8080/#/c/21031/26/fe/src/main/java/org/apache/impala/util/DebugUtils.java File fe/src/main/java/org/apache/impala/util/DebugUtils.java: http://gerrit.cloudera.org:8080/#/c/21031/26/fe/src/main/java/org/apache/impala/util/DebugUtils.java@99 PS26, Line 99: public static final String EVENT_PROCESSOR = "event_processor"; nit: EVENT_PROCESSOR_STATE sounds more intuitive to me. http://gerrit.cloudera.org:8080/#/c/21031/26/fe/src/test/java/org/apache/impala/catalog/events/SynchronousHMSEventProcessorForTests.java File fe/src/test/java/org/apache/impala/catalog/events/SynchronousHMSEventProcessorForTests.java: http://gerrit.cloudera.org:8080/#/c/21031/26/fe/src/test/java/org/apache/impala/catalog/events/SynchronousHMSEventProcessorForTests.java@31 PS26, Line 31: pollingFrequencyInMilliSec Why change polling frequency to millisec when hierarchical event processor is not used? http://gerrit.cloudera.org:8080/#/c/21031/26/tests/custom_cluster/test_event_processing_perf.py File tests/custom_cluster/test_event_processing_perf.py: http://gerrit.cloudera.org:8080/#/c/21031/26/tests/custom_cluster/test_event_processing_perf.py@310 PS26, Line 310: ''' Why is this commented? http://gerrit.cloudera.org:8080/#/c/21031/26/tests/custom_cluster/test_event_processing_perf.py@352 PS26, Line 352: .format(drop_db_table_time[0], drop_db_table_time[1])) Should compare the performance with and without a hierarchical event processor instead of logging the timing? http://gerrit.cloudera.org:8080/#/c/21031/26/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21031/26/tests/custom_cluster/test_events_custom_configs.py@1345 PS26, Line 1345: self.cluster.catalogd.set_jvm_log_level("org.apache.impala.util.DebugUtils", "trace") Why do we need to set the log level to trace? -- To view, visit http://gerrit.cloudera.org:8080/21031 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I76d8a739f9db6d40f01028bfd786a85d83f9e5d6 Gerrit-Change-Number: 21031 Gerrit-PatchSet: 26 Gerrit-Owner: Anonymous Coward <k.venureddy2...@gmail.com> Gerrit-Reviewer: Anonymous Coward <cclive1...@gmail.com> Gerrit-Reviewer: Anonymous Coward <k.venureddy2...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Thu, 14 Nov 2024 20:30:23 +0000 Gerrit-HasComments: Yes
