Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/20533 )
Change subject: PROTOTYPE: IMPALA-12463: Batch non-consecutive table events in the event processor ...................................................................... Patch Set 3: (3 comments) looks good! I agree with the behavior, the comments are about making it easier to understand http://gerrit.cloudera.org:8080/#/c/20533/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/20533/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@327 PS3, Line 327: TreeMap<Long, MetastoreEvent> Is my understanding correct that the map is only needed because batchable events are added as single events first, and converted to batch events only after there is a second event? It may simplify things to convert to alter/insert partition events to batch events as soon as we reach them in the loop if there is no previous batch event for the table. This way sortedBatchedEvents could be a simple list of events and we could add the current event to the end of it instantly without waiting for the tableEventMap to be flushed. I am ok with not doing this refactor, just thought that it would make the code easier to understand. http://gerrit.cloudera.org:8080/#/c/20533/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@346 PS3, Line 346: while (it.hasNext()) { Some of the logic could be moved to helper functions to make the main loop easier to understand, e.g. flushBatchEventsForDb() and flushBatchEventsForTable() http://gerrit.cloudera.org:8080/#/c/20533/3/fe/src/main/java/org/apache/impala/catalog/events/MetastoreEvents.java@368 PS3, Line 368: // Flush any batched events for the destination table. A note could be added about this being an invalid case, as the dest table shouldn't have existed before the rename, so there shouldn't be batched events for it. -- 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: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[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: Sat, 07 Oct 2023 15:05:18 +0000 Gerrit-HasComments: Yes
