From d0107bc1d8a9298fdefe75ff878cf9f09783cb35 Mon Sep 17 00:00:00 2001 From: winkyao Date: Thu, 10 May 2018 20:41:30 +0800 Subject: [PATCH] ddl: fix bug that update handle failed before adding table indics (#6523) --- ddl/ddl_db_test.go | 16 ++++++++++++++++ ddl/index.go | 11 +---------- ddl/reorg.go | 26 +++++++++++++++++++++++++- ddl/reorg_test.go | 18 ++++++++++-------- 4 files changed, 52 insertions(+), 19 deletions(-) diff --git a/ddl/ddl_db_test.go b/ddl/ddl_db_test.go index ecb2ac5b09..4485817df6 100644 --- a/ddl/ddl_db_test.go +++ b/ddl/ddl_db_test.go @@ -23,6 +23,7 @@ import ( "sync" "time" + gofail "github.com/coreos/gofail/runtime" "github.com/juju/errors" . "github.com/pingcap/check" "github.com/pingcap/tidb/ddl" @@ -1885,3 +1886,18 @@ func (s *testDBSuite) TestAddNotNullColumnWhileInsertOnDupUpdate(c *C) { wg.Wait() c.Assert(tk2Err, IsNil) } + +func (s *testDBSuite) TestUpdateHandleFailed(c *C) { + gofail.Enable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle", `return(true)`) + defer gofail.Disable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle") + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_handle_failed") + defer tk.MustExec("drop database test_handle_failed") + tk.MustExec("use test_handle_failed") + tk.MustExec("create table t(a int primary key, b int)") + tk.MustExec("insert into t values(-1, 1)") + tk.MustExec("alter table t add index idx_b(b)") + result := tk.MustQuery("select count(*) from t use index(idx_b)") + result.Check(testkit.Rows("1")) + tk.MustExec("admin check index t idx_b") +} diff --git a/ddl/index.go b/ddl/index.go index 12854e97b6..d77417e942 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -262,17 +262,8 @@ func (d *ddl) onCreateIndex(t *meta.Meta, job *model.Job) (ver int64, err error) } var reorgInfo *reorgInfo - reorgInfo, err = d.getReorgInfo(t, job) + reorgInfo, err = d.getReorgInfo(t, job, tbl) if err != nil || reorgInfo.first { - if err == nil { - // Get the first handle of this table. - err = iterateSnapshotRows(d.store, tbl, reorgInfo.SnapshotVer, math.MinInt64, - func(h int64, rowKey kv.Key, rawRecord []byte) (bool, error) { - reorgInfo.Handle = h - return false, nil - }) - return ver, errors.Trace(t.UpdateDDLReorgHandle(reorgInfo.Job, reorgInfo.Handle)) - } // If we run reorg firstly, we should update the job snapshot version // and then run the reorg next time. return ver, errors.Trace(err) diff --git a/ddl/reorg.go b/ddl/reorg.go index 6bf8f3d984..53721a847e 100644 --- a/ddl/reorg.go +++ b/ddl/reorg.go @@ -14,6 +14,7 @@ package ddl import ( + "math" "sync/atomic" "time" @@ -23,6 +24,7 @@ import ( "github.com/pingcap/tidb/model" "github.com/pingcap/tidb/mysql" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/util/mock" log "github.com/sirupsen/logrus" ) @@ -172,7 +174,9 @@ type reorgInfo struct { first bool } -func (d *ddl) getReorgInfo(t *meta.Meta, job *model.Job) (*reorgInfo, error) { +var gofailOnceGuard bool + +func (d *ddl) getReorgInfo(t *meta.Meta, job *model.Job, tbl table.Table) (*reorgInfo, error) { var err error info := &reorgInfo{ @@ -191,6 +195,26 @@ func (d *ddl) getReorgInfo(t *meta.Meta, job *model.Job) (*reorgInfo, error) { return nil, errInvalidStoreVer.Gen("invalid storage current version %d", ver.Ver) } + // Get the first handle of this table. + err = iterateSnapshotRows(d.store, tbl, ver.Ver, math.MinInt64, + func(h int64, rowKey kv.Key, rawRecord []byte) (bool, error) { + info.Handle = h + return false, nil + }) + if err != nil { + return info, errors.Trace(err) + } + // gofail: var errorUpdateReorgHandle bool + // if errorUpdateReorgHandle && !gofailOnceGuard { + // // only return error once. + // gofailOnceGuard = true + // return info, errors.New("occur an error when update reorg handle.") + // } + err = t.UpdateDDLReorgHandle(job, info.Handle) + if err != nil { + return info, errors.Trace(err) + } + job.SnapshotVer = ver.Ver } else { info.Handle, err = t.GetDDLReorgHandle(job) diff --git a/ddl/reorg_test.go b/ddl/reorg_test.go index c8b9960cad..42324a6cd4 100644 --- a/ddl/reorg_test.go +++ b/ddl/reorg_test.go @@ -69,7 +69,7 @@ func (s *testDDLSuite) TestReorg(c *C) { } job := &model.Job{ ID: 1, - SnapshotVer: 1, // Make sure it is not zero. So the reorgInfo's frist is false. + SnapshotVer: 1, // Make sure it is not zero. So the reorgInfo's first is false. } err = ctx.NewTxn() c.Assert(err, IsNil) @@ -93,8 +93,9 @@ func (s *testDDLSuite) TestReorg(c *C) { c.Assert(err, IsNil) err = ctx.NewTxn() c.Assert(err, IsNil) + m = meta.NewMeta(ctx.Txn()) - info, err1 := d.getReorgInfo(m, job) + info, err1 := d.getReorgInfo(m, job, nil) c.Assert(err1, IsNil) c.Assert(info.Handle, Equals, handle) c.Assert(d.reorgCtx.doneHandle, Equals, int64(0)) @@ -114,17 +115,18 @@ func (s *testDDLSuite) TestReorg(c *C) { d.start(context.Background()) job = &model.Job{ - ID: 2, - SchemaID: 1, - Type: model.ActionCreateSchema, - Args: []interface{}{model.NewCIStr("test")}, + ID: 2, + SchemaID: 1, + Type: model.ActionCreateSchema, + Args: []interface{}{model.NewCIStr("test")}, + SnapshotVer: 1, // Make sure it is not zero. So the reorgInfo's first is false. } var info *reorgInfo err = kv.RunInNewTxn(d.store, false, func(txn kv.Transaction) error { t := meta.NewMeta(txn) var err1 error - info, err1 = d.getReorgInfo(t, job) + info, err1 = d.getReorgInfo(t, job, nil) c.Assert(err1, IsNil) err1 = info.UpdateHandle(txn, 1) c.Assert(err1, IsNil) @@ -135,7 +137,7 @@ func (s *testDDLSuite) TestReorg(c *C) { err = kv.RunInNewTxn(d.store, false, func(txn kv.Transaction) error { t := meta.NewMeta(txn) var err1 error - info, err1 = d.getReorgInfo(t, job) + info, err1 = d.getReorgInfo(t, job, nil) c.Assert(err1, IsNil) c.Assert(info.Handle, Greater, int64(0)) return nil