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 8: (2 comments) 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@577 PS7, Line 577: throws MetastoreNotificationException > It is possible that the new impala client can talk to the old HMS where the Sorry that I'm not clear how the MetastoreNotificationException will be thrown here. This is a function declaration, not an error handling that catch any exception. Impala could talk to old HMS. But it should use the HMS client that it compiles with. And the HMS client seems won't thrown any exceptions here. To be specifit, the invoked methods don't throw exceptions: MetastoreEventsProcessor#getMessageDeserializer() https://github.com/apache/impala/blob/9ebe8ccdaa8a7e1c671beabb30597e089917482e/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEventsProcessor.java#L1083 MessageDeserializer#getCommitCompactionMessage() https://github.com/apache/hive/blob/fcf80448513f17c48cfac5ff5cf613e93dd313f2/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/MessageDeserializer.java#L245 NotificationEvent#getMessage() https://github.com/apache/hive/blob/fcf80448513f17c48cfac5ff5cf613e93dd313f2/standalone-metastore/metastore-common/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/NotificationEvent.java#L324 CommitCompactionMessage#getPartName() https://github.com/apache/hive/blob/fcf80448513f17c48cfac5ff5cf613e93dd313f2/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/messaging/CommitCompactionMessage.java#L46 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 > There is no method in CatalogServiceCatalog that would only refresh the fil I see. I thought we can use MetastoreTableEvent#reloadTableFromCatalog() for unpartitioned tables and MetastoreTableEvent#reloadPartitions() for partitioned tables. But reloadTableFromCatalog() will reload the table schema. reloadPartitions() requires the hmsPartition objects. Here we just have the partition names. So they don't fit all the requirements here. However, I think we do have methods that reload the file metadata only, e.g. HdfsTable#reloadPartitionsFromNames() https://github.com/apache/impala/blob/9ebe8ccdaa8a7e1c671beabb30597e089917482e/fe/src/main/java/org/apache/impala/catalog/HdfsTable.java#L2752 Such methods are more mature since they handle some corner cases, e.g. if the partition is already removed in HMS while processing the event. So I suggest adding a reloadPartitionsForNames method in MetastoreTableEvent and let CommitCompactionEvent extends MetastoreTableEvent to reuse it. We can move the current codes of refreshFileMetadataForCommitCompaction() into it, and use HdfsTable#reloadPartitionsFromNames() instead. -- 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: 8 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, 31 Jan 2023 12:27:11 +0000 Gerrit-HasComments: Yes
