ddl: fix parallel create table if not exists #6286
Conversation
ddl/ddl_db_change_test.go
Outdated
| @@ -433,7 +433,6 @@ func (s *testStateChangeSuite) TestShowIndex(c *C) { | |||
| } | |||
|
|
|||
| func (s *testStateChangeSuite) TestParallelDDL(c *C) { | |||
| defer testleak.AfterTest(c)() | |||
There was a problem hiding this comment.
It's called in testStateChangeSuite.TeareDownSuite, call testleak.AfterTest(c) here make test unstable.
ddl/ddl_db_change_test.go
Outdated
| @@ -501,3 +499,85 @@ func (s *testStateChangeSuite) TestParallelDDL(c *C) { | |||
| callback = &ddl.TestDDLCallback{} | |||
| d.SetHook(callback) | |||
| } | |||
|
|
|||
| func (s *testStateChangeSuite) TestCreateIfNotExists(c *C) { | |||
There was a problem hiding this comment.
Add some comments for the test case.
ddl/ddl_db_change_test.go
Outdated
| d.SetHook(callback) | ||
| } | ||
|
|
||
| func (s *testStateChangeSuite) TestCreateIfNotExists1(c *C) { |
ddl/ddl_db_change_test.go
Outdated
| @@ -501,3 +499,87 @@ func (s *testStateChangeSuite) TestParallelDDL(c *C) { | |||
| callback = &ddl.TestDDLCallback{} | |||
| d.SetHook(callback) | |||
| } | |||
|
|
|||
| // TestCreateIfNotExists parallel exec create table if not exists xxx. No error returns is expected. | |||
| func (s *testStateChangeSuite) TestCreateIfNotExists(c *C) { | |||
There was a problem hiding this comment.
s/TestCreateIfNotExists/TestCreateTableIfNotExists
ddl/ddl_db_change_test.go
Outdated
| } | ||
|
|
||
| // TestCreateIfNotExists parallel exec create database if not exists xxx. No error returns is expected. | ||
| func (s *testStateChangeSuite) TestCreateIfNotExists1(c *C) { |
There was a problem hiding this comment.
/TestCreateIfNotExists1/TestCreateDBIfNotExists
ddl/ddl_db_change_test.go
Outdated
| wg.Add(2) | ||
| go func() { | ||
| defer wg.Done() | ||
| _, err2 = se.Execute(context.Background(), "create table if not exists test_not_exists(a int);") |
There was a problem hiding this comment.
This test is similar to the next test, only the SQL is different. And we will add some test like this. Could this be extracted into a function?
…nto correct_ifnotexists_check
|
@zimulala PTAL |
ddl/ddl_db_change_test.go
Outdated
| callback := &ddl.TestDDLCallback{} | ||
| once := sync.Once{} | ||
| callback.OnJobUpdatedExported = func(job *model.Job) { | ||
| // sleep a while, let other job enque. |
There was a problem hiding this comment.
How can we confirm that the other job is in the queue?
There was a problem hiding this comment.
Before this PR, the added test failed stably.
There was a problem hiding this comment.
But this test code is indeed unstable.
shenli
added
the
priority/release-blocker
ddl/ddl_api.go
Outdated
| @@ -856,6 +856,11 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e | |||
| err = d.handleAutoIncID(tbInfo, schema.ID) | |||
| } | |||
| } | |||
|
|
|||
| // table exists, but if_not_exits flags is true, so we ignore this error. | |||
…nto correct_ifnotexists_check
|
@zimulala @lamxTyler PTAL |
|
/run-all-tests |
In onCreateTable, we will check if table is exists, but we use the
if not existsoption only in CreateTable. It will cause unexpected[schema:1050]Table 'Xxx' already exists