ddl: refine ddl error when do ddl error cause by infoschema is changed #11308
Conversation
crazycs520
added
component/DDL-need-LGT3
type/enhancement
Codecov Report
@@ Coverage Diff @@
## master #11308 +/- ##
===============================================
- Coverage 81.3031% 81.2601% -0.043%
===============================================
Files 423 423
Lines 90641 90241 -400
===============================================
- Hits 73694 73330 -364
+ Misses 11632 11611 -21
+ Partials 5315 5300 -15 |
ddl/db_test.go
Outdated
| hook.OnJobRunBeforeExported = func(job *model.Job) { | ||
| if job.Type == model.ActionTruncateTable && job.State == model.JobStateNone { | ||
| wg.Add(1) | ||
| go func() { |
There was a problem hiding this comment.
Is it possible that the truncate table is executed before the add column operation?
ddl/ddl.go
Outdated
| @@ -582,6 +582,7 @@ func (d *ddl) doDDLJob(ctx sessionctx.Context, job *model.Job) error { | |||
| return errors.Trace(err) | |||
| } | |||
| ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue = true | |||
| defer func() { ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue = false }() | |||
There was a problem hiding this comment.
IsDDLJobInQueue is used to mark whether the DDL job is put into the queue.
This originally refers to whether the DDL job has been placed in the queue, and the description may not be very clear. This variable was originally used to handle scenarios that encountered this problem before being placed on the queue.
There was a problem hiding this comment.
ok. I will recover this logic.
ddl/db_test.go
Outdated
| @@ -3387,6 +3387,38 @@ func (s *testDBSuite2) TestDDLWithInvalidTableInfo(c *C) { | |||
| c.Assert(err.Error(), Equals, "[parser:1064]You have an error in your SQL syntax; check the manual that corresponds to your TiDB version for the right syntax to use line 1 column 94 near \"then (b / a) end));\" ") | |||
| } | |||
|
|
|||
| // TestParallelTruncateTableAndAddColumn tests add column when truncate table. | |||
| func (s *testDBSuite5) TestParallelTruncateTableAndAddColumn(c *C) { | |||
There was a problem hiding this comment.
Could we put it to db_change_test.go? The parallel DDL tests are placed in this file.
…into refine-ddl-error
|
/run-all-tests |
|
/run-all-tests |
1 similar comment
|
/run-all-tests |
|
/run-all-tests |
|
/run-unit-test |
| return e.toErr(err) | ||
| // If owner return ErrTableNotExists error when running this DDL, It maybe cause by schema changed, | ||
| // otherwise, ErrTableNotExists could return before put this DDL job to job queue. | ||
| if (e.ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue && infoschema.ErrTableNotExists.Equal(err)) || |
There was a problem hiding this comment.
How about we execute drop table at the same table in parallel? Add the two DDL statements all pass the check, and generate the DDL jobs. Then the second job needs to return ErrTableNotExists, but in this logical, it seems to return Information schema is changed.
There was a problem hiding this comment.
yep...This is a compromise. If we want to fix this problem, maybe we need a retry or other step to make this better.
executor/ddl.go
Outdated
| if e.ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue { | ||
| return err | ||
| } | ||
|
|
||
| // Before the DDL job is ready, it encouters an error that may be due to the outdated schema information. | ||
| // After the DDL job is ready, the ErrInfoSchemaChanged error won't happen because we are getting the schema directly from storage. |
There was a problem hiding this comment.
Do we update these comments?
crazycs520
added
status/LGT3
What problem does this PR solve?
Considering running the following 2 DDL statements in parallel:
If "
truncate table" is prior to "alter table" in the DDL job queue, "alter table" will return "Table '(Schema ID 1).(Table ID 41)' doesn't exist", the error message is confusing. "Information schema is changed" may be a better choice.What is changed and how it works?
toErrlogic in executor/ddl.goCheck List
Tests
Code changes
Side effects
Related changes