Skip to content

Commit 68b529e

Browse files
lance6716lance6716
authored
ddl: replace OnJobUpdated callback with failpoint (#55091)
ref #54436
1 parent 8c7abde commit 68b529e

38 files changed

+257
-576
lines changed

‎pkg/ddl/callback.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,6 @@ type Callback interface {
3232
OnJobRunBefore(job *model.Job)
3333
// OnJobRunAfter is called after running job.
3434
OnJobRunAfter(job *model.Job)
35-
// OnJobUpdated is called after the running job is updated.
36-
OnJobUpdated(job *model.Job)
3735
}
3836

3937
// BaseCallback implements Callback.OnChanged interface.
@@ -50,11 +48,6 @@ func (*BaseCallback) OnJobRunAfter(_ *model.Job) {
5048
// Nothing to do.
5149
}
5250

53-
// OnJobUpdated implements Callback.OnJobUpdated interface.
54-
func (*BaseCallback) OnJobUpdated(_ *model.Job) {
55-
// Nothing to do.
56-
}
57-
5851
// SchemaLoader is used to avoid import loop, the only impl is domain currently.
5952
type SchemaLoader interface {
6053
Reload() error

‎pkg/ddl/cancel_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -272,14 +272,14 @@ func TestCancel(t *testing.T) {
272272

273273
resetHook := func(h *callback.TestDDLCallback) {
274274
h.OnJobRunBeforeExported = nil
275-
h.OnJobUpdatedExported.Store(nil)
275+
_ = failpoint.Disable("github.com/pingcap/tidb/pkg/ddl/onJobUpdated")
276276
dom.DDL().SetHook(h.Clone())
277277
}
278278
registerHook := func(h *callback.TestDDLCallback, onJobRunBefore bool) {
279279
if onJobRunBefore {
280280
h.OnJobRunBeforeExported = hookFunc
281281
} else {
282-
h.OnJobUpdatedExported.Store(&hookFunc)
282+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", hookFunc)
283283
}
284284
dom.DDL().SetHook(h.Clone())
285285
}

‎pkg/ddl/cluster_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,6 @@ func TestGlobalVariablesOnFlashback(t *testing.T) {
203203

204204
func TestCancelFlashbackCluster(t *testing.T) {
205205
store, dom := testkit.CreateMockStoreAndDomain(t)
206-
originHook := dom.DDL().GetHook()
207206
tk := testkit.NewTestKit(t, store)
208207

209208
time.Sleep(10 * time.Millisecond)
@@ -223,7 +222,7 @@ func TestCancelFlashbackCluster(t *testing.T) {
223222
hook := newCancelJobHook(t, store, dom, func(job *model.Job) bool {
224223
return job.SchemaState == model.StateDeleteOnly
225224
})
226-
dom.DDL().SetHook(hook)
225+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", hook.OnJobUpdated)
227226
tk.MustExec("set global tidb_ttl_job_enable = on")
228227
tk.MustGetErrCode(fmt.Sprintf("flashback cluster to timestamp '%s'", oracle.GetTimeFromTS(ts).Format(types.TimeFSPFormat)), errno.ErrCancelledDDLJob)
229228
hook.MustCancelDone(t)
@@ -236,16 +235,14 @@ func TestCancelFlashbackCluster(t *testing.T) {
236235
hook = newCancelJobHook(t, store, dom, func(job *model.Job) bool {
237236
return job.SchemaState == model.StateWriteReorganization
238237
})
239-
dom.DDL().SetHook(hook)
238+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", hook.OnJobUpdated)
240239
tk.MustExec(fmt.Sprintf("flashback cluster to timestamp '%s'", oracle.GetTimeFromTS(ts).Format(types.TimeFSPFormat)))
241240
hook.MustCancelFailed(t)
242241

243242
rs, err = tk.Exec("show variables like 'tidb_ttl_job_enable'")
244243
assert.NoError(t, err)
245244
assert.Equal(t, tk.ResultSetToResult(rs, "").Rows()[0][1], variable.Off)
246245

247-
dom.DDL().SetHook(originHook)
248-
249246
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/ddl/mockFlashbackTest"))
250247
require.NoError(t, failpoint.Disable("github.com/pingcap/tidb/pkg/ddl/injectSafeTS"))
251248
}

‎pkg/ddl/column_change_test.go

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/pingcap/tidb/pkg/tablecodec"
3535
"github.com/pingcap/tidb/pkg/testkit"
3636
"github.com/pingcap/tidb/pkg/testkit/external"
37+
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
3738
"github.com/pingcap/tidb/pkg/types"
3839
"github.com/pingcap/tidb/pkg/util/mock"
3940
"github.com/stretchr/testify/require"
@@ -61,7 +62,7 @@ func TestColumnAdd(t *testing.T) {
6162
)
6263
first := true
6364
var jobID int64
64-
onJobUpdatedExportedFunc := func(job *model.Job) {
65+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
6566
jobID = job.ID
6667
tbl, exist := dom.InfoSchema().TableByID(job.TableID)
6768
require.True(t, exist)
@@ -79,9 +80,7 @@ func TestColumnAdd(t *testing.T) {
7980
publicTable = tbl
8081
require.NoError(t, checkAddPublic(ct, writeOnlyTable, publicTable))
8182
}
82-
}
83-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
84-
d.SetHook(tc.Clone())
83+
})
8584
tk.MustExec("alter table t add column c3 int default 3")
8685
tb := publicTable
8786
v := getSchemaVer(t, tk.Session())
@@ -94,7 +93,7 @@ func TestColumnAdd(t *testing.T) {
9493
dropCol = tbl.VisibleCols()[2]
9594
}
9695
}
97-
onJobUpdatedExportedFunc2 := func(job *model.Job) {
96+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
9897
if job.NotStarted() {
9998
return
10099
}
@@ -105,8 +104,7 @@ func TestColumnAdd(t *testing.T) {
105104
require.NotEqualf(t, col.ID, dropCol.ID, "column is not dropped")
106105
}
107106
}
108-
}
109-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc2)
107+
})
110108
d.SetHook(tc.Clone())
111109
tk.MustExec("alter table t drop column c3")
112110
v = getSchemaVer(t, tk.Session())
@@ -115,7 +113,7 @@ func TestColumnAdd(t *testing.T) {
115113

116114
// Add column not default.
117115
first = true
118-
onJobUpdatedExportedFunc3 := func(job *model.Job) {
116+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
119117
jobID = job.ID
120118
tbl, exist := dom.InfoSchema().TableByID(job.TableID)
121119
require.True(t, exist)
@@ -133,9 +131,7 @@ func TestColumnAdd(t *testing.T) {
133131
_, err = writeOnlyTable.AddRecord(sess.GetTableCtx(), types.MakeDatums(10, 10))
134132
require.NoError(t, err)
135133
}
136-
}
137-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc3)
138-
d.SetHook(tc)
134+
})
139135
tk.MustExec("alter table t add column c3 int")
140136
testCheckJobDone(t, store, jobID, true)
141137
}

‎pkg/ddl/column_modify_test.go

Lines changed: 9 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import (
3737
"github.com/pingcap/tidb/pkg/table/tables"
3838
"github.com/pingcap/tidb/pkg/testkit"
3939
"github.com/pingcap/tidb/pkg/testkit/external"
40+
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
4041
"github.com/pingcap/tidb/pkg/types"
4142
"github.com/pingcap/tidb/pkg/util/mock"
4243
"github.com/stretchr/testify/require"
@@ -477,7 +478,7 @@ func TestTransactionWithWriteOnlyColumn(t *testing.T) {
477478

478479
// For issue #31735.
479480
func TestAddGeneratedColumnAndInsert(t *testing.T) {
480-
store, dom := testkit.CreateMockStoreAndDomainWithSchemaLease(t, columnModifyLease)
481+
store := testkit.CreateMockStoreWithSchemaLease(t, columnModifyLease)
481482

482483
tk := testkit.NewTestKit(t, store)
483484
tk.MustExec("use test")
@@ -487,13 +488,11 @@ func TestAddGeneratedColumnAndInsert(t *testing.T) {
487488
tk1 := testkit.NewTestKit(t, store)
488489
tk1.MustExec("use test")
489490

490-
d := dom.DDL()
491-
hook := &callback.TestDDLCallback{Do: dom}
492491
ctx := mock.NewContext()
493492
ctx.Store = store
494493
times := 0
495494
var checkErr error
496-
onJobUpdatedExportedFunc := func(job *model.Job) {
495+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
497496
if checkErr != nil {
498497
return
499498
}
@@ -517,26 +516,23 @@ func TestAddGeneratedColumnAndInsert(t *testing.T) {
517516
times++
518517
}
519518
}
520-
}
521-
hook.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
522-
d.SetHook(hook)
519+
})
523520

524521
tk.MustExec("alter table t1 add column gc int as ((a+1))")
525522
tk.MustQuery("select * from t1 order by a").Check(testkit.Rows("4 5", "10 11"))
526523
require.NoError(t, checkErr)
527524
}
528525

529526
func TestColumnTypeChangeGenUniqueChangingName(t *testing.T) {
530-
store, dom := testkit.CreateMockStoreAndDomainWithSchemaLease(t, columnModifyLease)
527+
store := testkit.CreateMockStoreWithSchemaLease(t, columnModifyLease)
531528

532529
tk := testkit.NewTestKit(t, store)
533530
tk.MustExec("use test")
534531

535-
hook := &callback.TestDDLCallback{}
536532
var checkErr error
537533
assertChangingColName := "_col$_c2_0"
538534
assertChangingIdxName := "_idx$_idx_0"
539-
onJobUpdatedExportedFunc := func(job *model.Job) {
535+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
540536
if job.SchemaState == model.StateDeleteOnly && job.Type == model.ActionModifyColumn {
541537
var (
542538
_newCol *model.ColumnInfo
@@ -559,10 +555,7 @@ func TestColumnTypeChangeGenUniqueChangingName(t *testing.T) {
559555
checkErr = errors.New("changing index name is incorrect")
560556
}
561557
}
562-
}
563-
hook.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
564-
d := dom.DDL()
565-
d.SetHook(hook)
558+
})
566559

567560
tk.MustExec("create table if not exists t(c1 varchar(256), c2 bigint, `_col$_c2` varchar(10), unique _idx$_idx(c1), unique idx(c2));")
568561
tk.MustExec("alter table test.t change column c2 cC2 tinyint after `_col$_c2`")
@@ -593,7 +586,7 @@ func TestColumnTypeChangeGenUniqueChangingName(t *testing.T) {
593586
assertChangingColName2 := "_col$__col$__col$_c1_0_1"
594587
query1 := "alter table t modify column _col$_c1 tinyint"
595588
query2 := "alter table t modify column _col$__col$_c1_0 tinyint"
596-
onJobUpdatedExportedFunc2 := func(job *model.Job) {
589+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
597590
if (job.Query == query1 || job.Query == query2) && job.SchemaState == model.StateDeleteOnly && job.Type == model.ActionModifyColumn {
598591
var (
599592
_newCol *model.ColumnInfo
@@ -616,9 +609,7 @@ func TestColumnTypeChangeGenUniqueChangingName(t *testing.T) {
616609
checkErr = errors.New("changing column name is incorrect")
617610
}
618611
}
619-
}
620-
hook.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc2)
621-
d.SetHook(hook)
612+
})
622613

623614
tk.MustExec("drop table if exists t")
624615
tk.MustExec("create table if not exists t(c1 bigint, _col$_c1 bigint, _col$__col$_c1_0 bigint, _col$__col$__col$_c1_0_0 bigint)")

‎pkg/ddl/column_test.go

Lines changed: 9 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"github.com/pingcap/tidb/pkg/table/tables"
3636
"github.com/pingcap/tidb/pkg/tablecodec"
3737
"github.com/pingcap/tidb/pkg/testkit"
38+
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
3839
"github.com/pingcap/tidb/pkg/types"
3940
"github.com/stretchr/testify/require"
4041
)
@@ -643,8 +644,6 @@ func testGetColumn(t table.Table, name string, isExist bool) error {
643644
func TestAddColumn(t *testing.T) {
644645
store, dom := testkit.CreateMockStoreAndDomain(t, mockstore.WithDDLChecker())
645646

646-
d := dom.DDL()
647-
648647
tk := testkit.NewTestKit(t, store)
649648
tk.MustExec("use test")
650649
tk.MustExec("create table t1 (c1 int, c2 int, c3 int);")
@@ -672,8 +671,7 @@ func TestAddColumn(t *testing.T) {
672671

673672
checkOK := false
674673

675-
tc := &callback.TestDDLCallback{Do: dom}
676-
onJobUpdatedExportedFunc := func(job *model.Job) {
674+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
677675
if checkOK {
678676
return
679677
}
@@ -689,9 +687,7 @@ func TestAddColumn(t *testing.T) {
689687
if newCol.State == model.StatePublic {
690688
checkOK = true
691689
}
692-
}
693-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
694-
d.SetHook(tc)
690+
})
695691

696692
jobID := testCreateColumn(tk, t, testkit.NewTestKit(t, store).Session(), tableID, newColName, "", defaultColValue, dom)
697693
testCheckJobDone(t, store, jobID, true)
@@ -705,8 +701,6 @@ func TestAddColumn(t *testing.T) {
705701
func TestAddColumns(t *testing.T) {
706702
store, dom := testkit.CreateMockStoreAndDomain(t, mockstore.WithDDLChecker())
707703

708-
d := dom.DDL()
709-
710704
tk := testkit.NewTestKit(t, store)
711705
tk.MustExec("use test")
712706
tk.MustExec("create table t1 (c1 int, c2 int, c3 int);")
@@ -740,8 +734,7 @@ func TestAddColumns(t *testing.T) {
740734
err = txn.Commit(context.Background())
741735
require.NoError(t, err)
742736

743-
tc := &callback.TestDDLCallback{Do: dom}
744-
onJobUpdatedExportedFunc := func(job *model.Job) {
737+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
745738
mu.Lock()
746739
defer mu.Unlock()
747740
if checkOK {
@@ -761,9 +754,7 @@ func TestAddColumns(t *testing.T) {
761754
checkOK = true
762755
}
763756
}
764-
}
765-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
766-
d.SetHook(tc)
757+
})
767758

768759
jobID := testCreateColumns(tk, t, testkit.NewTestKit(t, store).Session(), tableID, newColNames, positions, defaultColValue, dom)
769760

@@ -809,9 +800,7 @@ func TestDropColumnInColumnTest(t *testing.T) {
809800
var hookErr error
810801
var mu sync.Mutex
811802

812-
d := dom.DDL()
813-
tc := &callback.TestDDLCallback{Do: dom}
814-
onJobUpdatedExportedFunc := func(job *model.Job) {
803+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
815804
mu.Lock()
816805
defer mu.Unlock()
817806
if checkOK {
@@ -823,9 +812,7 @@ func TestDropColumnInColumnTest(t *testing.T) {
823812
checkOK = true
824813
return
825814
}
826-
}
827-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
828-
d.SetHook(tc)
815+
})
829816

830817
jobID := testDropColumnInternal(tk, t, testkit.NewTestKit(t, store).Session(), tableID, colName, false, dom)
831818
testCheckJobDone(t, store, jobID, false)
@@ -871,9 +858,7 @@ func TestDropColumns(t *testing.T) {
871858
var hookErr error
872859
var mu sync.Mutex
873860

874-
d := dom.DDL()
875-
tc := &callback.TestDDLCallback{Do: dom}
876-
onJobUpdatedExportedFunc := func(job *model.Job) {
861+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
877862
mu.Lock()
878863
defer mu.Unlock()
879864
if checkOK {
@@ -887,9 +872,7 @@ func TestDropColumns(t *testing.T) {
887872
return
888873
}
889874
}
890-
}
891-
tc.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
892-
d.SetHook(tc)
875+
})
893876

894877
jobID := testDropColumns(tk, t, testkit.NewTestKit(t, store).Session(), tableID, colNames, false, dom)
895878
testCheckJobDone(t, store, jobID, false)

‎pkg/ddl/column_type_change_test.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import (
3434
"github.com/pingcap/tidb/pkg/tablecodec"
3535
"github.com/pingcap/tidb/pkg/testkit"
3636
"github.com/pingcap/tidb/pkg/testkit/external"
37+
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
3738
"github.com/pingcap/tidb/pkg/util"
3839
"github.com/pingcap/tidb/pkg/util/dbterror"
3940
"github.com/stretchr/testify/require"
@@ -305,7 +306,7 @@ func TestRowLevelChecksumWithMultiSchemaChange(t *testing.T) {
305306
// It's good because the insert / update logic will cast the related column to changing column rather than use
306307
// origin default value directly.
307308
func TestChangingColOriginDefaultValue(t *testing.T) {
308-
store, dom := testkit.CreateMockStoreAndDomain(t)
309+
store := testkit.CreateMockStore(t)
309310
tk := testkit.NewTestKit(t, store)
310311
tk.MustExec("set global tidb_enable_row_level_checksum = 1")
311312
tk.MustExec("use test")
@@ -319,14 +320,12 @@ func TestChangingColOriginDefaultValue(t *testing.T) {
319320
tk.MustExec("insert into t values(2, 2)")
320321

321322
tbl := external.GetTableByName(t, tk, "test", "t")
322-
originalHook := dom.DDL().GetHook()
323-
hook := &callback.TestDDLCallback{Do: dom}
324323
var (
325324
once bool
326325
checkErr error
327326
)
328327
i := 0
329-
onJobUpdatedExportedFunc := func(job *model.Job) {
328+
testfailpoint.EnableCall(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated", func(job *model.Job) {
330329
if checkErr != nil {
331330
return
332331
}
@@ -371,11 +370,9 @@ func TestChangingColOriginDefaultValue(t *testing.T) {
371370
}
372371
i++
373372
}
374-
}
375-
hook.OnJobUpdatedExported.Store(&onJobUpdatedExportedFunc)
376-
dom.DDL().SetHook(hook)
373+
})
377374
tk.MustExec("alter table t modify column b tinyint NOT NULL")
378-
dom.DDL().SetHook(originalHook)
375+
testfailpoint.Disable(t, "github.com/pingcap/tidb/pkg/ddl/onJobUpdated")
379376
require.NoError(t, checkErr)
380377
// Since getReorgInfo will stagnate StateWriteReorganization for a ddl round, so insert should exec 3 times.
381378
tk.MustQuery("select * from t order by a").Check(testkit.Rows("1 -1", "2 -2", "3 3", "4 4", "5 5"))

0 commit comments

Comments
 (0)