Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/19155 )
Change subject: IMPALA-11626: Handle COMMIT_COMPACTION_EVENT from HMS ...................................................................... Patch Set 10: (7 comments) http://gerrit.cloudera.org:8080/#/c/19155/10/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/19155/10/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2653 PS10, Line 2653: private boolean isPartitionEmpty_ = false; unused variable http://gerrit.cloudera.org:8080/#/c/19155/10/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2695 PS10, Line 2695: return false; I think we should reuse the implementation in MetastoreTableEvent instead of overriding it as always return false. So tables or databases that have 'impala.disableHmsSync' set to 'true' in properties can be skipped in event processing. http://gerrit.cloudera.org:8080/#/c/19155/10/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/19155/10/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4521 PS10, Line 4521: reloaded. nit: move this to the above line http://gerrit.cloudera.org:8080/#/c/19155/9/fe/src/test/resources/hive-site.xml.py File fe/src/test/resources/hive-site.xml.py: http://gerrit.cloudera.org:8080/#/c/19155/9/fe/src/test/resources/hive-site.xml.py@167 PS9, Line 167: # Since HIVE-22589, Hive uses Julian Calendar for writing dates before 1582-10-15, > Yeah, this would probably introduce a data-loading performance issue. Inste Thanks for the explanation! Could you comment this in the new test? http://gerrit.cloudera.org:8080/#/c/19155/10/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/19155/10/tests/custom_cluster/test_events_custom_configs.py@268 PS10, Line 268: 5 Could you comment why we need such a long inverval (5s)? I think the reason is the scenario of compact and drop a partition. We want to process the compaction event after the partition is dropped. http://gerrit.cloudera.org:8080/#/c/19155/10/tests/custom_cluster/test_events_custom_configs.py@284 PS10, Line 284: # self.run_stmt_in_hive( : # "insert into {}.{} partition (year=2022) values (4),(5),(6)" : # .format(unique_database, test_cc_part_table)) Can we remove commented codes? http://gerrit.cloudera.org:8080/#/c/19155/10/tests/custom_cluster/test_events_custom_configs.py@287 PS10, Line 287: parts_refreshed_before_compaction = EventProcessorUtils.get_int_metric( Should we call EventProcessorUtils.wait_for_event_processing(self) before this? I think we should wait until all the insert events are processed. -- To view, visit http://gerrit.cloudera.org:8080/19155 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I464faedb4e3bbcd417bab2e3cb0d57e339d42605 Gerrit-Change-Number: 19155 Gerrit-PatchSet: 10 Gerrit-Owner: Sai Hemanth Gantasala <saihema...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@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: Sat, 25 Feb 2023 14:03:43 +0000 Gerrit-HasComments: Yes
