Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17308 )
Change subject: IMPALA-10502: Handle CREATE/DROP events correctly ...................................................................... Patch Set 5: (4 comments) 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@1773 PS5, Line 1773: // this means that the database was recreated from some other application while : // this create database operation was in progress. We bail out by throwing an error : // in this case because it is possible the database which the user was trying to : // create was not the one which was eventually recreated in the metastore. : Preconditions.checkState(events.size() == 1, : "Database was recreated in metastore while " : + "createDatabase operation was in progress"); > I'm thinking whether this is too strict. Maybe it's ok to not fail the ddl yeah, it is slightly a tricky case to handle. Since metastore will only allow one database with the given name at any point of time, the only reason why this condition would satisfy would be if the database was dropped and recreated since the eventId on line 1700. So consider the scenario where we have 2 events here. This means since the eventId on line 1700 (assume it is 100) we saw 101:CREATE_DATABASE, 102:DROP_DATABASE, 103:CREATE_DATABASE events. Ideally if event 103 is the one which this operation created, we are good. But if the db which was created by this operation was dropped and recreated by event 103, then there could be variations like missing dbproperties, locationUri etc from the perspective of the user. I think it makes sense to try to recover from this when if not exists is provided. We can simply get the database from the last event if, if not exists clause is provided. I can make that change. http://gerrit.cloudera.org:8080/#/c/17308/5/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@3107 PS5, Line 3107: it is possible that Kudu doesn't generate : // any metastore events. > Do you mean Kudu could use a createTable API that doesn't generate HMS even Yeah, I am not sure how this works really. I noticed that on some cases, a create table API to kudu didn't generate any notifications. I am not sure why that would be the case. I can investigate more. http://gerrit.cloudera.org:8080/#/c/17308/4/tests/custom_cluster/test_event_processing.py File tests/custom_cluster/test_event_processing.py: http://gerrit.cloudera.org:8080/#/c/17308/4/tests/custom_cluster/test_event_processing.py@286 PS4, Line 286: e > flake8: E722 do not use bare except' Done http://gerrit.cloudera.org:8080/#/c/17308/4/tests/custom_cluster/test_event_processing.py@301 PS4, Line 301: > flake8: E226 missing whitespace around arithmetic operator Done -- 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 <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Sourabh Goyal <[email protected]> Gerrit-Reviewer: Vihang Karajgaonkar <[email protected]> Gerrit-Comment-Date: Thu, 22 Apr 2021 23:30:21 +0000 Gerrit-HasComments: Yes
