Vihang Karajgaonkar 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 > My thought process behind this change was: Thanks for the explanation. The approach only slightly simplifies the DDL code path in the assumed scenario where DDLs sync to latest event id. e.g. for(MetastoreEvent event : metastoreEvents) { event.processIfEnabled(); } v/s for(MetastoreEvent event : metastoreEvents) { event.processIfEnabled(); addToDeleteEventLog(event); } While on the flip side, it complicates the events processor path now where we seem to unnecessarily add event in the deleteLog which is anyways going to be garbage collected. This could reduce debuggability of the code. Also, it seems that on the event code path, we are overwriting the same event in the deleteEventLog which is counter intuitive. In my opinion the benefits are not significant enough to outweigh added complexity. Thoughts? -- 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 <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Sourabh Goyal <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Mon, 02 Aug 2021 21:04:20 +0000 Gerrit-HasComments: Yes
