-
Notifications
You must be signed in to change notification settings - Fork 5.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ddl: implement the core for multi-schema change #35429
Changes from all commits
0aa52df
dd93dcc
29e9a22
acae418
ded9c2e
8a45600
31e13d4
306465d
91968e4
7d2c42a
32880da
3ca3ee1
d14b995
285ab3a
9204d4b
1a1ef47
fc2c4d3
46776ac
58f1cdb
fc53f91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -712,6 +712,12 @@ func setDDLJobQuery(ctx sessionctx.Context, job *model.Job) { | |
// - context.Cancel: job has been sent to worker, but not found in history DDL job before cancel | ||
// - other: found in history DDL job and return that job error | ||
func (d *ddl) DoDDLJob(ctx sessionctx.Context, job *model.Job) error { | ||
if mci := ctx.GetSessionVars().StmtCtx.MultiSchemaInfo; mci != nil { | ||
xiongjiwei marked this conversation as resolved.
Show resolved
Hide resolved
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the ddl_api level, please add a parallel processing test: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean the test like this? This test puts 2 multi-schema change DDLs to the job queue at the same time. func TestMultiSchemaChangeAddColumnsParallel(t *testing.T) {
store, clean := testkit.CreateMockStore(t)
defer clean()
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("create table t (a int default 1);")
tk.MustExec("insert into t values ();")
putTheSameDDLJobTwice(t, func() {
tk.MustExec("alter table t add column if not exists b int default 2, " +
"add column if not exists c int default 3;")
tk.MustQuery("show warnings").Check(testkit.Rows(
"Note 1060 Duplicate column name 'b'",
"Note 1060 Duplicate column name 'c'",
))
})
tk.MustQuery("select * from t;").Check(testkit.Rows("1 2 3"))
tk.MustExec("drop table if exists t;")
tk.MustExec("create table t (a int);")
putTheSameDDLJobTwice(t, func() {
tk.MustGetErrCode("alter table t add column b int, add column c int;", errno.ErrDupFieldName)
})
} Let me add it in the next PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what "putTheSameDDLJobTwice" does, but it's a similar test |
||
// In multiple schema change, we don't run the job. | ||
// Instead, we merge all the jobs into one pending job. | ||
return appendToSubJobs(mci, job) | ||
} | ||
|
||
// Get a global job ID and put the DDL job in the queue. | ||
setDDLJobQuery(ctx, job) | ||
task := &limitJobTask{job, make(chan error)} | ||
|
@@ -786,12 +792,7 @@ func (d *ddl) DoDDLJob(ctx sessionctx.Context, job *model.Job) error { | |
} | ||
} | ||
} | ||
|
||
if historyJob.MultiSchemaInfo != nil && len(historyJob.MultiSchemaInfo.Warnings) != 0 { | ||
for _, warning := range historyJob.MultiSchemaInfo.Warnings { | ||
ctx.GetSessionVars().StmtCtx.AppendWarning(warning) | ||
} | ||
} | ||
appendMultiChangeWarningsToOwnerCtx(ctx, historyJob) | ||
|
||
logutil.BgLogger().Info("[ddl] DDL job is finished", zap.Int64("jobID", jobID)) | ||
return nil | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is duplicated with line106-114? Could we use it for line106-114?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. For now, line106-114 is only used by
onAddColumns()
(the plural version), which will be removed in the following PRs.