Vihang Karajgaonkar has posted comments on this change. ( http://gerrit.cloudera.org:8080/14319 )
Change subject: IMPALA-8984: Fix race condition in creating Kudu table ...................................................................... Patch Set 13: > > > > (1 comment) > > > > > > Lock both kudu table creation and hms table creation by > > 'DdlLock_' > > > can certainly solve this problem, but maybe to heavy. > > > It doesn't matter that KuduCatalogOpExecutor.createManagedTable > > is > > > not thread-safey, because the root reason for this bug is: > create > > > table in hms failed(first query already created) caused an > > > exception which lead to table delete in kudu storage(line > 2254). > > > Even two threads create table in kudu stoage at the same time, > > kudu > > > provide guarantee to this situation, and it's doesn't matter > > which > > > query created table in kudu finally. So I just add the code to > > > check whether table already exist in hms. > > > > I think what I meant was if there are 2 threads which try to > create > > a Kudu table with the same name, one of them might succeed while > > other will fail at > > https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/service/KuduCatalogOpExecutor.java#L101 > > > > This error will be thrown as ImpalaRuntimeException which is will > > fail the query. If the query specifies create if not exists, > > ideally it should be idempotent. It may not be the exact problem > > that this patch is fixing but it is close enough to the race > > condition described and it would be good to handle this case as > > well. > > Thanks for your kindly review and reply, Vihang. I understand, you > mean two thread create same table, and 'kudu.createTable' is not > thread-safety, this may caused ImpalaRuntimeException without 'if > not exists'. I've already read the related code, and here is my > opion: > Firstly, not only create, but drop/rename/alter all exist the > problem you mentioned above, we need to add lock in all these > situation. And for these situations, more test cases need to be > written to verify the function, is it better to deal with these > issues in another jira? > Secondly, this bug is not very similar to those issues > describe(kudu create/drop/rename/alter), maybe 'race condition' is > not very exactly for this jira. > How do you think? > Besides, I'm not sure that my fe test is suitable. Quanlong Huang > suggest me to write an e2e test by python. But I cannot reporduce > this bug with an e2e test case, due to thread schedule path are > random, only specific path can lead to this bug, so I've no idea > about this. I am okay with fixing the other race condition with respect to kudu create table statement erroring out if that works better for you. -- To view, visit http://gerrit.cloudera.org:8080/14319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a4047bcdaa6b346765b96e8c36bb747a2b0091d Gerrit-Change-Number: 14319 Gerrit-PatchSet: 13 Gerrit-Owner: wangsheng <sky...@163.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Reviewer: wangsheng <sky...@163.com> Gerrit-Comment-Date: Thu, 19 Dec 2019 18:47:51 +0000 Gerrit-HasComments: No
