ddl: add recover for run ddl job #10981

crazycs520 · 2019-06-28T08:14:26Z

What problem does this PR solve?

Fix ddl worker run ddl job panic.

[2019/06/28 15:21:54.379 +08:00] [INFO] [ddl_worker.go:474] ["[ddl] run DDL job"] [worker="worker 1, tp general"] [job="ID:48, Type:create table, State:none, SchemaState:none, SchemaID:1, TableID:47, RowCount:0, ArgLen:0, start time: 2019-06-28 15:19:12.653 +0800 CST, Err:<nil>, ErrCount:0, SnapshotVersion:0"]
[2019/06/28 15:21:54.379 +08:00] [ERROR] [ddl.go:441] ["[ddl] DDL worker meet panic"] [worker="worker 1, tp general"] [ID=8c8b0d58-4238-4f21-bd15-6915af71a983] [stack="github.com/pingcap/tidb/ddl.(*ddl).start.func2
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl.go:441
github.com/pingcap/tidb/util.WithRecovery.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:72
runtime.gopanic
	/usr/local/go1.12/src/runtime/panic.go:522
runtime.panicmem
	/usr/local/go1.12/src/runtime/panic.go:82
runtime.sigpanic
	/usr/local/go1.12/src/runtime/signal_unix.go:390
github.com/pingcap/tidb/infoschema.(*Handle).Get
	/Users/cs/code/goread/src/github.com/pingcap/tidb/infoschema/infoschema.go:302
github.com/pingcap/tidb/ddl.onCreateTable
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/table.go:56
github.com/pingcap/tidb/ddl.(*worker).runDDLJob
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:499
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:402
github.com/pingcap/tidb/kv.RunInNewTxn
	/Users/cs/code/goread/src/github.com/pingcap/tidb/kv/txn.go:50
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:365
github.com/pingcap/tidb/ddl.(*worker).start
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:137
github.com/pingcap/tidb/ddl.(*ddl).start.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl.go:438
github.com/pingcap/tidb/util.WithRecovery
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:80"]
[2019/06/28 15:21:54.380 +08:00] [ERROR] [misc.go:75] ["panic in the recoverable goroutine"] [r="\"invalid memory address or nil pointer dereference\""] ["stack trace"="github.com/pingcap/tidb/util.WithRecovery.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:77
runtime.gopanic
	/usr/local/go1.12/src/runtime/panic.go:522
runtime.panicmem
	/usr/local/go1.12/src/runtime/panic.go:82
runtime.sigpanic
	/usr/local/go1.12/src/runtime/signal_unix.go:390
github.com/pingcap/tidb/infoschema.(*Handle).Get
	/Users/cs/code/goread/src/github.com/pingcap/tidb/infoschema/infoschema.go:302
github.com/pingcap/tidb/ddl.onCreateTable
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/table.go:56
github.com/pingcap/tidb/ddl.(*worker).runDDLJob
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:499
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:402
github.com/pingcap/tidb/kv.RunInNewTxn
	/Users/cs/code/goread/src/github.com/pingcap/tidb/kv/txn.go:50
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:365
github.com/pingcap/tidb/ddl.(*worker).start
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:137
github.com/pingcap/tidb/ddl.(*ddl).start.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl.go:438
github.com/pingcap/tidb/util.WithRecovery
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:80"] [stack="github.com/pingcap/tidb/util.WithRecovery.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:75
runtime.gopanic
	/usr/local/go1.12/src/runtime/panic.go:522
runtime.panicmem
	/usr/local/go1.12/src/runtime/panic.go:82
runtime.sigpanic
	/usr/local/go1.12/src/runtime/signal_unix.go:390
github.com/pingcap/tidb/infoschema.(*Handle).Get
	/Users/cs/code/goread/src/github.com/pingcap/tidb/infoschema/infoschema.go:302
github.com/pingcap/tidb/ddl.onCreateTable
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/table.go:56
github.com/pingcap/tidb/ddl.(*worker).runDDLJob
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:499
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:402
github.com/pingcap/tidb/kv.RunInNewTxn
	/Users/cs/code/goread/src/github.com/pingcap/tidb/kv/txn.go:50
github.com/pingcap/tidb/ddl.(*worker).handleDDLJobQueue
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:365
github.com/pingcap/tidb/ddl.(*worker).start
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl_worker.go:137
github.com/pingcap/tidb/ddl.(*ddl).start.func1
	/Users/cs/code/goread/src/github.com/pingcap/tidb/ddl/ddl.go:438
github.com/pingcap/tidb/util.WithRecovery
	/Users/cs/code/goread/src/github.com/pingcap/tidb/util/misc.go:80"]

What is changed and how it works?

Add a recover in runDDLJob function.
check nil point.

Check List

Tests

Unit test

Code changes

Has exported function/method change

Side effects

Related changes

Need to cherry-pick to the release branch

codecov · 2019-06-28T08:19:49Z

Codecov Report

Merging #10981 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #10981   +/-   ##
===========================================
  Coverage   81.0665%   81.0665%           
===========================================
  Files           421        421           
  Lines         89577      89577           
===========================================
  Hits          72617      72617           
  Misses        11725      11725           
  Partials       5235       5235

ddl/table.go

ddl/serial_test.go

ddl/ddl_worker.go

ddl/serial_test.go

zimulala · 2019-06-28T09:16:29Z

ddl/ddl_worker.go

@@ -471,6 +472,18 @@ func chooseLeaseTime(t, max time.Duration) time.Duration {

 // runDDLJob runs a DDL job. It returns the current schema version in this transaction and the error.
 func (w *worker) runDDLJob(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) {
+	defer func() {
+		r := recover()


Could we use WithRecovery？

winkyao · 2019-06-28T09:18:55Z

ddl/ddl_worker.go

+			logutil.BgLogger().Error("run ddl job panic",
+				zap.Reflect("r", r),
+				zap.Stack("stack trace"))
+			job.State = model.JobStateCancelling


add some comments, why cancel the job here.

infoschema/infoschema.go

zimulala

LGTM

crazycs520 · 2019-07-01T09:34:44Z

/run-all-tests

winkyao

LGTM

bb7133 · 2019-07-01T16:19:55Z

LGTM

crazycs520 · 2019-07-02T00:50:58Z

/run-all-tests

crazycs520 · 2019-07-02T00:57:14Z

/run-unit-test

ddl: add recover for run ddl job

ed7edb0

crazycs520 added the component/DDL-need-LGT3 label Jun 28, 2019

crazycs520 added the type/bugfix This PR fixes a bug. label Jun 28, 2019

zimulala reviewed Jun 28, 2019

View reviewed changes

ddl/table.go Outdated Show resolved Hide resolved

zimulala reviewed Jun 28, 2019

View reviewed changes

ddl/serial_test.go Outdated Show resolved Hide resolved

zimulala reviewed Jun 28, 2019

View reviewed changes

ddl/ddl_worker.go Outdated Show resolved Hide resolved

amyangfei reviewed Jun 28, 2019

View reviewed changes

ddl/serial_test.go Outdated Show resolved Hide resolved

crazycs520 added 2 commits June 28, 2019 16:51

address comment

fe17a4a

address comment

912a642

zimulala reviewed Jun 28, 2019

View reviewed changes

winkyao reviewed Jun 28, 2019

View reviewed changes

infoschema/infoschema.go Show resolved Hide resolved

address comment

8a2f415

zimulala reviewed Jul 1, 2019

View reviewed changes

Merge branch 'master' into fix-ddl-panic

407cb62

winkyao reviewed Jul 1, 2019

View reviewed changes

Merge branch 'master' into fix-ddl-panic

8bdc4d7

bb7133 added the status/LGT2 Indicates that a PR has LGTM 2. label Jul 1, 2019

crazycs520 added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 2, 2019

winkyao approved these changes Jul 2, 2019

View reviewed changes

winkyao merged commit 6542e4a into pingcap:master Jul 2, 2019

crazycs520 added a commit to crazycs520/tidb that referenced this pull request Jul 2, 2019

ddl: add recover for run ddl job (pingcap#10981)

46634c4

crazycs520 mentioned this pull request Jul 2, 2019

ddl: add recover for run ddl job (#10981) #11022

Merged

winkyao pushed a commit that referenced this pull request Jul 2, 2019

ddl: add recover for run ddl job (#10981) (#11022)

e04723a

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ddl: add recover for run ddl job #10981

ddl: add recover for run ddl job #10981

crazycs520 commented Jun 28, 2019

codecov bot commented Jun 28, 2019 •

edited

Loading

zimulala Jun 28, 2019

crazycs520 Jul 1, 2019

winkyao Jun 28, 2019 •

edited

Loading

crazycs520 Jul 1, 2019

zimulala left a comment

crazycs520 commented Jul 1, 2019

winkyao left a comment

bb7133 commented Jul 1, 2019

crazycs520 commented Jul 2, 2019

crazycs520 commented Jul 2, 2019

ddl: add recover for run ddl job #10981

ddl: add recover for run ddl job #10981

Conversation

crazycs520 commented Jun 28, 2019

What problem does this PR solve?

What is changed and how it works?

Check List

codecov bot commented Jun 28, 2019 • edited Loading

Codecov Report

zimulala Jun 28, 2019

Choose a reason for hiding this comment

crazycs520 Jul 1, 2019

Choose a reason for hiding this comment

winkyao Jun 28, 2019 • edited Loading

Choose a reason for hiding this comment

crazycs520 Jul 1, 2019

Choose a reason for hiding this comment

zimulala left a comment

Choose a reason for hiding this comment

crazycs520 commented Jul 1, 2019

winkyao left a comment

Choose a reason for hiding this comment

bb7133 commented Jul 1, 2019

crazycs520 commented Jul 2, 2019

crazycs520 commented Jul 2, 2019

codecov bot commented Jun 28, 2019 •

edited

Loading

winkyao Jun 28, 2019 •

edited

Loading