Sourabh Goyal has posted comments on this change. ( http://gerrit.cloudera.org:8080/17308 )
Change subject: IMPALA-10502: Handle CREATE/DROP events correctly ...................................................................... Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/catalog/events/DeleteEventLog.java File fe/src/main/java/org/apache/impala/catalog/events/DeleteEventLog.java: http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/catalog/events/DeleteEventLog.java@106 PS5, Line 106: eventLog_ = new TreeMap<>(eventLog_.tailMap(eventId + 1)); nit: Add a debug log statement when doing garbage collection ? http://gerrit.cloudera.org:8080/#/c/17308/5/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/17308/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@744 PS5, Line 744: LOG.debug("EventId: {} Table was not added since it already exists in catalog", nit: Log table name as well? http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2257 PS5, Line 2257: .getNextMetastoreEvents(catalog_, eventId, Does it make more sense to either use the existing metastore client created at line no. 2247 or close it so that it is returned to the pool and let MetastoreEventsProcessor.getNextMetastoreEvents() get a client from pool ? http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2260 PS5, Line 2260: .equals(event.getEventType()) || DropTableEvent.DROP_TABLE_EVENT_TYPE Why are we filtering DROP_TABLE_EVENT_TYPE here? Isn't it being handled at line no. 2548? http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3093 PS5, Line 3093: msClient.getHiveClient().createTable(newTable); Just confirming: For any ddl operations, HMS adds entry to NotificationEvents table before returning the response to the client? If not, we may have an issue when executing getNextMetastoreEvents(). -- To view, visit http://gerrit.cloudera.org:8080/17308 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia2c5e96b48abac015240f20295b3ec3b1d71f24a Gerrit-Change-Number: 17308 Gerrit-PatchSet: 5 Gerrit-Owner: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Sourabh Goyal <soura...@cloudera.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Tue, 11 May 2021 17:09:32 +0000 Gerrit-HasComments: Yes
