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 <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: wangsheng <[email protected]>
Gerrit-Comment-Date: Sun, 03 Nov 2019 23:15:19 +0000
Gerrit-HasComments: Yes

Reply via email to