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 14: (5 comments) The patch looks pretty good now. Just comments on the case when the table is unloaded. http://gerrit.cloudera.org:8080/#/c/19155/14//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19155/14//COMMIT_MSG@13 PS14, Line 13: processiing nit: processing http://gerrit.cloudera.org:8080/#/c/19155/14/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/14/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2664 PS14, Line 2664: msTbl_ = tbl.getMetaStoreTable(); Just realized that msTbl_ could be null which will break MetastoreTableEvent#isEventProcessingDisabled(). We need to override isEventProcessingDisabled() and check whether msTbl_ is null first, e.g. https://github.com/apache/impala/blob/7c854e117be659abdfed4ead43df75e5903d9132/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java#L2419-L2421 http://gerrit.cloudera.org:8080/#/c/19155/14/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/14/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@4524 PS14, Line 4524: returns 0 nit: returns the number of partitions reloaded Please also correct the comment in the above method. http://gerrit.cloudera.org:8080/#/c/19155/14/tests/custom_cluster/test_events_custom_configs.py File tests/custom_cluster/test_events_custom_configs.py: http://gerrit.cloudera.org:8080/#/c/19155/14/tests/custom_cluster/test_events_custom_configs.py@275 PS14, Line 275: # Will run same queries on two partitions to avoid flakiness stale comment? We just insert to the save partition. http://gerrit.cloudera.org:8080/#/c/19155/14/tests/custom_cluster/test_events_custom_configs.py@339 PS14, Line 339: assert EventProcessorUtils.get_event_processor_status() == "ACTIVE" Can we test the case when the table is unloaded? Just want to cover the case when msTbl_ of CommitCompactionEvent is null. -- 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: 14 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: Tue, 28 Feb 2023 12:12:39 +0000 Gerrit-HasComments: Yes
