[Impala-ASF-CR] IMPALA-8984: Fix race condition in creating Kudu table

["Quanlong Huang (Code Review)" <ge...@cloudera.org>]

Quanlong Huang 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 6:

(1 comment)

> Patch Set 6:
> 
> > (8 comments)
>  > 
>  > Nice find! I can understand the bug, but have some questions for
>  > the patch.
> 
> Hi, Quanlong, I'm trying to add a fe test to reproduce this situation. The test mainly work like this: First create a kudu managed table, then deleted it from catalog, still reserved in metastore. And submit the same ddl request again, and the original code would lead to table been deleted in kudu storage, and thus we would got an incompleted table when invalidated table. With this patch fixed, we would got an normal kudu table.

Sorry for my late reply. I think adding an e2e test may be better and easier. E.g. tests/stress/test_ddl_stress.py or the test_invalidation_races added in tests/custom_cluster/test_local_catalog.py for IMPALA-7534. The purpose is that the new tests should fail without your fix.

Commands to run these two tests:

 cd $IMPALA_HOME
 source bin/impala-config.sh
 tests/run-tests.py --exploration_strategy=exhaustive stress/test_ddl_stress.py
 impala-py.test tests/custom_cluster/test_local_catalog.py -k test_invalidation_races

More docs: https://cwiki.apache.org/confluence/display/IMPALA/How+to+load%2C+run%2C+and+create+new+Impala+tests

http://gerrit.cloudera.org:8080/#/c/14319/3/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/14319/3/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java@2170
PS3, Line 2170:       KuduCatalogOpExecutor.createManagedTable(newTable, params);
> The problem you mentioned above just like: kudu already exists a table, and
Nice find! I think we don't need to fix it in this patch. Please create a JIRA for this and comment a TODO with the JIRA and a short reason, e.g. "// TODO(IMPALA-xxxx): handle existing kudu table with a different schema"



-- 
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: 6
Gerrit-Owner: wangsheng <sk...@163.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Reviewer: wangsheng <sk...@163.com>
Gerrit-Comment-Date: Sun, 03 Nov 2019 23:15:19 +0000
Gerrit-HasComments: Yes