Skip to content
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: fix ddl hang over when it meets panic in cancelling path #23204

Merged
merged 18 commits into from
Mar 13, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions ddl/column_type_change_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,21 @@
package ddl_test

import (
"context"
"errors"
"time"

. "github.com/pingcap/check"
errors2 "github.com/pingcap/errors"
"github.com/pingcap/failpoint"
"github.com/pingcap/parser/model"
parser_mysql "github.com/pingcap/parser/mysql"
"github.com/pingcap/parser/terror"
"github.com/pingcap/tidb/ddl"
"github.com/pingcap/tidb/domain"
mysql "github.com/pingcap/tidb/errno"
"github.com/pingcap/tidb/kv"
"github.com/pingcap/tidb/meta"
"github.com/pingcap/tidb/session"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/store/helper"
Expand Down Expand Up @@ -1718,3 +1722,53 @@ func (s *testColumnTypeChangeSuite) TestChangingAttributeOfColumnWithFK(c *C) {

tk.MustExec("drop table if exists orders, users")
}

// Close issue #23202
func (s *testColumnTypeChangeSuite) TestDDLExitWhenCancelMeetPanic(c *C) {
AilinKid marked this conversation as resolved.
Show resolved Hide resolved
tk := testkit.NewTestKit(c, s.store)
tk.MustExec("use test")

tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a int, b int)")
tk.MustExec("insert into t values(1,1),(2,2)")
tk.MustExec("alter table t add index(b)")

failpoint.Enable("github.com/pingcap/tidb/ddl/mockExceedErrorLimit", `return(true)`)
defer func() {
failpoint.Disable("github.com/pingcap/tidb/ddl/mockExceedErrorLimit")
}()

originalHook := s.dom.DDL().GetHook()
defer s.dom.DDL().(ddl.DDLForTest).SetHook(originalHook)

hook := &ddl.TestDDLCallback{Do: s.dom}
var jobID int64
hook.OnJobRunBeforeExported = func(job *model.Job) {
if jobID != 0 {
return
}
if "drop index" == job.Type.String() {
AilinKid marked this conversation as resolved.
Show resolved Hide resolved
jobID = job.ID
}
}
s.dom.DDL().(ddl.DDLForTest).SetHook(hook)

// when it panics in write-reorg state, the job will be pulled up as a cancelling job. Since drop-index with
// write-reorg can't be cancelled, so it will be converted to running state and try again (dead loop).
_, err := tk.Exec("alter table t drop index b")
c.Assert(err, NotNil)
c.Assert(err.Error(), Equals, "[ddl:-1]panic in handling ddl logic and error count beyond the limitation 512, cancelled")
c.Assert(jobID > 0, Equals, true)

// Verification of the history job state.
var job *model.Job
err = kv.RunInNewTxn(context.Background(), s.store, false, func(ctx context.Context, txn kv.Transaction) error {
t := meta.NewMeta(txn)
var err1 error
job, err1 = t.GetHistoryDDLJob(jobID)
return errors2.Trace(err1)
})
c.Assert(err, IsNil)
c.Assert(job.ErrorCount, Equals, int64(513))
c.Assert(job.Error.Error(), Equals, "[ddl:-1]panic in handling ddl logic and error count beyond the limitation 512, cancelled")
}
14 changes: 14 additions & 0 deletions ddl/ddl_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,20 @@ func (w *worker) runDDLJob(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64,
func() {
// If run DDL job panic, just cancel the DDL jobs.
job.State = model.JobStateCancelling
job.ErrorCount++
AilinKid marked this conversation as resolved.
Show resolved Hide resolved

// Load global ddl variables.
if err1 := loadDDLVars(w); err1 != nil {
logutil.Logger(w.logCtx).Error("[ddl] load DDL global variable failed", zap.Error(err1))
}

errorCount := variable.GetDDLErrorCountLimit()
if job.ErrorCount > errorCount {
msg := fmt.Sprintf("panic in handling ddl logic and error count beyond the limitation %d, cancelled", errorCount)
logutil.Logger(w.logCtx).Warn(msg)
job.Error = toTError(errors.New(msg))
job.State = model.JobStateCancelled
AilinKid marked this conversation as resolved.
Show resolved Hide resolved
}
}, false)

// Mock for run ddl job panic.
Expand Down
6 changes: 6 additions & 0 deletions ddl/index.go
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,12 @@ func onDropIndex(t *meta.Meta, job *model.Job) (ver int64, _ error) {
// Set column index flag.
dropIndexColumnFlag(tblInfo, indexInfo)

failpoint.Inject("mockExceedErrorLimit", func(val failpoint.Value) {
if val.(bool) {
panic("panic test in cancelling add index")
}
})

tblInfo.Columns = tblInfo.Columns[:len(tblInfo.Columns)-len(dependentHiddenCols)]

ver, err = updateVersionAndTableInfoWithCheck(t, job, tblInfo, originalState != model.StateNone)
Expand Down