k.venureddy2...@gmail.com 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 48: (9 comments) http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java File fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java: http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@130 PS46, Line 130: ivat > nit: need one space Done http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@228 PS46, Line 228: eventsProcessor_.getMetrics()); > nit: Add an error message in case this fails Done http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@300 PS46, Line 300: , > nit: add spaces Done http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@311 PS46, Line 311: // Check if executor are unassigned for dbs and tables > nit: use assertNull() directly Done http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@681 PS46, Line 681: } > nit: please rename or comment this about what it counts for. Have refactored it and added the comment now. http://gerrit.cloudera.org:8080/#/c/21031/46/fe/src/test/java/org/apache/impala/catalog/events/EventExecutorServiceTest.java@692 PS46, Line 692: assertFalse(state.isCreateProcessed()); > nit: 'while' statement has empty body. Add a sleep? Done http://gerrit.cloudera.org:8080/#/c/21031/46/tests/custom_cluster/test_event_processing_perf.py File tests/custom_cluster/test_event_processing_perf.py: http://gerrit.cloudera.org:8080/#/c/21031/46/tests/custom_cluster/test_event_processing_perf.py@340 PS46, Line 340: > nit: It'd be helpful to add a new mode that doesn't synchronize the process Done. Have added a flag process_events_together to resume EP only after all events are generated. Except create/drop databases and tables, remaining operations are processed together at the end when the flag set to true. Excluded create database and tables because we load the tables on catalogd before any operation after creation. And drop tables and databases are excluded because, due to optimization in hierarchical event processing, all queued events are skipped from processing if drop event is also queued behind for processing. http://gerrit.cloudera.org:8080/#/c/21031/46/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21031/46/tests/custom_cluster/test_events_custom_configs.py@1512 PS46, Line 1512: e w > nit: what's the meaning of "hep" here? typo? I meant, hierarchical event processing. Have changed it to hierarchical now to avoid confusion. http://gerrit.cloudera.org:8080/#/c/21031/46/tests/custom_cluster/test_events_custom_configs.py@1592 PS46, Line 1592: def rename_table_with_hierarchical_event_process(self, src_db_name, target_db_name, > I might have missed it but do we have test coverage on BarrierEvent blocks Done -- 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: 48 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: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Comment-Date: Tue, 29 Apr 2025 09:45:02 +0000 Gerrit-HasComments: Yes
