Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/20648 )
Change subject: IMPALA-10987: Changing impala.disableHmsSync in Hive should not break event processing ...................................................................... Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/20648/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20648/1//COMMIT_MSG@22 PS1, Line 22: te l > I don't think that a warning is needed, it is enough to log skipping the ev Ack http://gerrit.cloudera.org:8080/#/c/20648/5/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/20648/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1535 PS5, Line 1535: long startNs = System.nanoTime(); : if (wasEventSyncTurnedOn()) { : handleEventSyncTurnedOn(); : } else { > nit: move this comment to L1543 ? Ack http://gerrit.cloudera.org:8080/#/c/20648/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1560 PS5, Line 1560: is > nit: "is" ? Ack http://gerrit.cloudera.org:8080/#/c/20648/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1563 PS5, Line 1563: if (catalogOpExecutor_.addTableIfNotRemovedLater(getEventId(), msTbl_)) { > If the db is just missing in the cache but exists in HMS, catalogOpExecutor I have added a test case for the above scenario. Since we process the create db event before the alter table event in the queue we will not have the event processor going into an error state. http://gerrit.cloudera.org:8080/#/c/20648/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1578 PS5, Line 1578: is turned on for this t > nit: "is turned on for this table" Ack http://gerrit.cloudera.org:8080/#/c/20648/5/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1583 PS5, Line 1583: the table with createEve > I think the table exists at this point but its createEventId >= this event Yeah, this else condition is really not required. I just wanted to make the if-else condition complete. I'll log the createEventId in the error message. -- To view, visit http://gerrit.cloudera.org:8080/20648 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I37055990be49e91462ebc98aa97009ca768a0072 Gerrit-Change-Number: 20648 Gerrit-PatchSet: 6 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-Comment-Date: Mon, 11 Dec 2023 23:07:40 +0000 Gerrit-HasComments: Yes
