ddl: fix bug that update handle failed before adding table indics (#6523)

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")
+}

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)

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)

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