Quanlong Huang 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: (3 comments) http://gerrit.cloudera.org:8080/#/c/21031/48/tests/custom_cluster/test_event_processing_perf.py File tests/custom_cluster/test_event_processing_perf.py: http://gerrit.cloudera.org:8080/#/c/21031/48/tests/custom_cluster/test_event_processing_perf.py@453 PS48, Line 453: if add_part_time is not None: nit: This if..elif.. clause is a bit confusing. Can we simply use if is_partitioned: ... else: ... ? http://gerrit.cloudera.org:8080/#/c/21031/48/tests/custom_cluster/test_event_processing_perf.py@488 PS48, Line 488: end_event_id = EventProcessorUtils.get_current_notification_id(self.hive_client) : start_time = time.time() : self.__ensure_events_processed() : return end_event_id - start_event_id, time.time() - start_time nit: The if-checks is not that straightforward to understand. Maybe the code is more readable if we extract these to be a method, e.g. __process_events_now(), and use it directly to replace the callsites on self.__process_events(start_event_id, True). Then we don't need the new parameter 'process_now' in __process_events(). http://gerrit.cloudera.org:8080/#/c/21031/48/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/21031/48/tests/custom_cluster/test_events_custom_configs.py@1521 PS48, Line 1521: # drop table t2, rename t1 to t2, load some data to t2 and create t1 from hive again nit: can we pause EP here and resume it at L1528 to make sure all these events are polled in a batch and processed together? -- 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 12:10:06 +0000 Gerrit-HasComments: Yes
