[Impala-ASF-CR] [WIP]: Add drop event id to deleteEventLog for drop db/table/partition events

["Sourabh Goyal (Code Review)" <ge...@cloudera.org>]

Sourabh Goyal has posted comments on this change. ( http://gerrit.cloudera.org:8080/17741 )

Change subject: [WIP]: Add drop event id to deleteEventLog for drop db/table/partition events
......................................................................


Patch Set 4:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17741/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/17741/4/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@873
PS4, Line 873: addToDeleteEventLog
> I don't understand this change. Perhaps you can explain me this better.
My thought process behind this change was:

While syncing to latest event ID, when processing drop db/table/partition events, we need to add them to delete entry log. Consider the following sequence of events from Impala shell (assuming we are syncing to latest event id for every DDL operation): 
1. at t1 -> create_table
2. at t2 -> drop_table 
3. at t3 -> create_table event received by event processor
here t3 > t2 > t1

Now after #2, table would be removed from cache. And since create_table event by event processor should be a no-op, we need to add table to deleteEntryLog after drop_table request. 

Since drop event is calling this method, I thought of extending its functionality to also add eventID to deleteEvent log so that from CatalogOpExecutor and MetastoreService handler's code for syncing to latest event id would simply look like 

 for(MetastoreEvent event : metastoreEvents) {
    event.processIfEnabled();
}

What happens when this method is called from EventProcessor - This would add event ID to deleteEntryLog as you rightly pointed out. But note that after every event gets processed by EventProcessor, it also gets garbage collected from deleteEventEntry log. Pasting snippet: 

for (MetastoreEvent event : filteredEvents) {
          event.processIfEnabled();
          deleteEventLog_.garbageCollect(event.getEventId());
          lastSyncedEventId_.set(event.eventId_);
        }
}

Please let me know if I misunderstood something or you think this is not the right approach.



-- 
To view, visit http://gerrit.cloudera.org:8080/17741
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iffa519a477e211891e7fa7118950628ec42b45d4
Gerrit-Change-Number: 17741
Gerrit-PatchSet: 4
Gerrit-Owner: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sourabh Goyal <so...@cloudera.com>
Gerrit-Reviewer: Vihang Karajgaonkar <vi...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Aug 2021 15:09:13 +0000
Gerrit-HasComments: Yes