Csaba Ringhofer 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 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/20648/3/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/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1556 PS3, Line 1556: wasEventSyncTurnedOn Wouldn't it be better to move this block earlier, before self event check? I am unsure about the handling of enabling metadata sync as self event. Assume the following scenario: 0. event processing starts disabled for a table 1. another component changes a random table prop with ALTER TABLE 2. before processing the event from 1 impala enables hms sync on the table 3. the event from 1 arrives, but catalogd skips it as hms sync was still disabled at that point 4. the event from 2 arrives, but it is skipped at it is a self event I think that at 4 catalogd should mark the table as stale. as it didn't process event from 1 http://gerrit.cloudera.org:8080/#/c/20648/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1557 PS3, Line 1557: // check if the table exists or not. 1) if the table doesn't exist create an I think that moving this block to a separate function like handleEventSyncTurnedOn() would make the main path clearer. http://gerrit.cloudera.org:8080/#/c/20648/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@1562 PS3, Line 1562: getTable Are we prepared for throwing an exception here? Using getTableNoThrow seems more fitting to avoid exception when the DB also doesn't exist (not sure if this is possible at this point) -- 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: 3 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: Thu, 07 Dec 2023 15:41:35 +0000 Gerrit-HasComments: Yes
