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 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/19155/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/19155/4//COMMIT_MSG@9 PS4, Line 9: HMS emits an event when a compaction is committed > It'll be a single commit compaction event always. Consider a partitioned ta I see. Thanks for the explanation! http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java@441 PS7, Line 441: public static String getFieldsFromReloadEvent(NotificationEvent event) Why do we need to change the return type of this? http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-apache-hive-3/java/org/apache/impala/compat/MetastoreShim.java@449 PS7, Line 449: NotificationEvent nit: our code style tend to put the type and var name at the same line. http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java File fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java: http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@570 PS7, Line 570: This method extracts the table, db, and partition fields from the : * notification event and returns them in a map. Please update this http://gerrit.cloudera.org:8080/#/c/19155/7/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java@577 PS7, Line 577: throws MetastoreNotificationException nit: it seems no exceptions will be thrown http://gerrit.cloudera.org:8080/#/c/19155/4/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/4/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@220 PS4, Line 220: case INSERT: > This event can come as a standalone event after a major/minor compaction ap Ack http://gerrit.cloudera.org:8080/#/c/19155/7/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/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@2632 PS7, Line 2632: MetastoreEvent I think we should extend MetastoreTableEvent so we can reuse its reload methods instead of adding a new one (refreshFileMetadataForCommitCompaction). -- 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: 7 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Comment-Date: Sun, 29 Jan 2023 03:04:50 +0000 Gerrit-HasComments: Yes
