Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20022 )
Change subject: IMPALA-11535: Skip older events in the event processor based on the latestRefreshEventID ...................................................................... Patch Set 5: (2 comments) http://gerrit.cloudera.org:8080/#/c/20022/4/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/20022/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@1527 PS4, Line 1527: reloadFileMetadata, reloadTableSchema, false, partitionsToUpdate, : debugAction, partitionToEventId, reason); > We always reloadFileMetadat, reloadTableSchema in catalogOpExecutor wheneve I am still trying to understand what will happen here. For example when calling from alterCommentOnColumn: https://github.com/apache/impala/blob/8329e6f2e39463d01b549ae84518d131b848adff/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java#L7315 This sets reloadFileMetadata to false, which is logical for a DDL that changes only column metadata. But what happens if there is an unprocessed insert event? Won't we set lastRefreshEventId to the latest event id (so after the insert event) and skip the insert event as a result? http://gerrit.cloudera.org:8080/#/c/20022/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java File fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java: http://gerrit.cloudera.org:8080/#/c/20022/4/fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java@3233 PS4, Line 3233: "test", null); : eventsProcessor_.processEvents(); : assertTrue(testTbl.getLastRefreshEventId() > Ack. we would want to compare currentEventId of event processor to be equal Wouldn't it be better to save the value of testTbl.getLastRefreshEventId() before eventsProcessor_.processEvents(); and compare with that afterwards? My concern is that if we do not skip the event, then the event processing will trigger a reload that also sets lastRefreshEventId_ so we do not verify that getOrLoadTable() actually sets lastRefreshEventId_ -- To view, visit http://gerrit.cloudera.org:8080/20022 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic0dc5c7396d80616680d8a5805ce80db293b72e1 Gerrit-Change-Number: 20022 Gerrit-PatchSet: 5 Gerrit-Owner: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 08 Aug 2023 09:30:57 +0000 Gerrit-HasComments: Yes
