Sai Hemanth Gantasala has posted comments on this change. ( http://gerrit.cloudera.org:8080/20533 )
Change subject: IMPALA-12463: Batch non-consecutive table events in the event processor ...................................................................... Patch Set 7: (4 comments) http://gerrit.cloudera.org:8080/#/c/20533/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/20533/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@400 PS7, Line 400: } else if (next instanceof AlterTableEvent) { Shouldn't we consider create/drop table events as batch-breaking events? http://gerrit.cloudera.org:8080/#/c/20533/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@405 PS7, Line 405: flushBatchForTable(pendingTableEventsMap, sortedFinalBatches, beforeTable); IMO, we should also consider the scenario where the rename table happens across different databases also and flush the corresponding events. Eg: Alter table rename db1.tb1 to db2.tb2; http://gerrit.cloudera.org:8080/#/c/20533/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@408 PS7, Line 408: // an invalid scenario, because the destination must not exist at the time This is a possible scenario, In the current queue, There are table events for t1, table events for t2, drop event for t1, rename event from t2 to t1. http://gerrit.cloudera.org:8080/#/c/20533/7/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@426 PS7, Line 426: dbMap = pendingTableEventsMap.get(dbName); Shouldn't we just assign a new HashMap<>() directly to the dbMap variable? -- To view, visit http://gerrit.cloudera.org:8080/20533 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I849c0306bc46080ee4059854f42d9c217a89b905 Gerrit-Change-Number: 20533 Gerrit-PatchSet: 7 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Sai Hemanth Gantasala <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 14 Dec 2023 01:15:49 +0000 Gerrit-HasComments: Yes
