Skip to content

Commit

Permalink
ddl: return error for canceled job (pingcap#54595)
Browse files Browse the repository at this point in the history
  • Loading branch information
GMHDBJD authored Jul 23, 2024
1 parent be01e5a commit d0c73aa
Show file tree
Hide file tree
Showing 5 changed files with 31 additions and 2 deletions.
1 change: 1 addition & 0 deletions pkg/ddl/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -1154,6 +1154,7 @@ func setDDLJobMode(job *model.Job) {
}
case model.ActionCreateSchema:
job.LocalMode = true
return
default:
}
job.LocalMode = false
Expand Down
13 changes: 13 additions & 0 deletions pkg/ddl/ddl_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -1223,6 +1223,19 @@ func (w *worker) HandleLocalDDLJob(d *ddlCtx, job *model.Job) (err error) {
if err != nil {
return err
}
// no need to rollback for fast create table now.
if job.IsCancelling() {
job.State = model.JobStateCancelled
job.Error = dbterror.ErrCancelledDDLJob
}
if job.IsCancelled() {
w.sess.Reset()
if err = w.handleJobDone(d, job, t); err != nil {
return err
}
// return job.Error to let caller know the job is cancelled.
return job.Error
}

d.mu.RLock()
d.mu.hook.OnJobRunAfter(job)
Expand Down
2 changes: 1 addition & 1 deletion pkg/ddl/job_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -494,7 +494,7 @@ func (d *ddl) delivery2LocalWorker(pool *workerPool, jobW *JobWrapper) {
}()

for i := int64(0); i < variable.GetDDLErrorCountLimit(); i++ {
err := wk.HandleLocalDDLJob(d.ddlCtx, job)
err = wk.HandleLocalDDLJob(d.ddlCtx, job)
// since local the job is not inserted into the ddl job queue, we need to add retry logic here.
if err == nil || !isRetryableError(err) {
break
Expand Down
3 changes: 2 additions & 1 deletion pkg/ddl/tests/fail/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ go_test(
],
flaky = True,
race = "on",
shard_count = 12,
shard_count = 13,
deps = [
"//pkg/config",
"//pkg/ddl",
Expand All @@ -24,6 +24,7 @@ go_test(
"//pkg/store/mockstore",
"//pkg/tablecodec",
"//pkg/testkit",
"//pkg/testkit/testfailpoint",
"//pkg/testkit/testsetup",
"@com_github_pingcap_failpoint//:failpoint",
"@com_github_stretchr_testify//require",
Expand Down
14 changes: 14 additions & 0 deletions pkg/ddl/tests/fail/fail_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import (
"github.com/pingcap/tidb/pkg/store/mockstore"
"github.com/pingcap/tidb/pkg/tablecodec"
"github.com/pingcap/tidb/pkg/testkit"
"github.com/pingcap/tidb/pkg/testkit/testfailpoint"
"github.com/stretchr/testify/require"
"github.com/tikv/client-go/v2/testutils"
"go.opencensus.io/stats/view"
Expand Down Expand Up @@ -334,6 +335,19 @@ func TestRunDDLJobPanicDisableClusteredIndex(t *testing.T) {
})
}

// TestRunDDLJobPanicEnableFastCreateTable tests recover panic with fast create table when run ddl job panic.
func TestRunDDLJobPanicEnableFastCreateTable(t *testing.T) {
s := createFailDBSuite(t)
tk := testkit.NewTestKit(t, s.store)
tk.MustExec("use test")
tk.MustExec("drop table if exists t")
tk.MustExec("set global tidb_enable_fast_create_table=ON")
testfailpoint.Enable(t, "github.com/pingcap/tidb/pkg/ddl/mockPanicInRunDDLJob", `1*panic("panic test")`)
_, err := tk.Exec("create table t(c1 int, c2 int)")
require.Error(t, err)
require.EqualError(t, err, "[ddl:8214]Cancelled DDL job")
}

func testAddIndexWorkerNum(t *testing.T, s *failedSuite, test func(*testkit.TestKit)) {
if variable.EnableDistTask.Load() {
t.Skip("dist reorg didn't support checkBackfillWorkerNum, skip this test")
Expand Down

0 comments on commit d0c73aa

Please sign in to comment.