From 37d503510b15f3863a9d5edbfed68797f160de7e Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 11 Dec 2019 20:22:13 +0800 Subject: [PATCH 1/2] ddl: fix ErrGCTooEarly when adding index to partition table --- ddl/failtest/fail_db_test.go | 21 +++++++++++++++++++++ ddl/index.go | 16 +++++++++++++++- ddl/reorg.go | 19 +++++++++++++------ 3 files changed, 49 insertions(+), 7 deletions(-) mode change 100644 => 100755 ddl/index.go mode change 100644 => 100755 ddl/reorg.go diff --git a/ddl/failtest/fail_db_test.go b/ddl/failtest/fail_db_test.go index 8488cda6cddf7..c5f610210ba3e 100644 --- a/ddl/failtest/fail_db_test.go +++ b/ddl/failtest/fail_db_test.go @@ -400,3 +400,24 @@ func (s *testFailDBSuite) TestRunDDLJobPanic(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[ddl:12]cancelled DDL job") } + +func (s *testFailDBSuite) TestPartitionAddIndexGC(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("use test") + tk.MustExec(`create table partition_add_idx ( + id int not null, + hired date not null + ) + partition by range( year(hired) ) ( + partition p1 values less than (1991), + partition p5 values less than (2008), + partition p7 values less than (2018) + );`) + tk.MustExec("insert into partition_add_idx values(1, '2010-01-01'), (2, '1990-01-01'), (3, '2001-01-01')") + + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/mockUpdateCachedSafePoint", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/mockUpdateCachedSafePoint"), IsNil) + }() + tk.MustExec("alter table partition_add_idx add index idx (id, hired)") +} diff --git a/ddl/index.go b/ddl/index.go old mode 100644 new mode 100755 index 71912361e8d30..a6f93f5a70268 --- a/ddl/index.go +++ b/ddl/index.go @@ -36,6 +36,7 @@ import ( "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/store/tikv" + "github.com/pingcap/tidb/store/tikv/oracle" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" @@ -1423,7 +1424,20 @@ func (w *worker) updateReorgInfo(t table.PartitionedTable, reorg *reorgInfo) (bo return true, nil } - start, end, err := getTableRange(reorg.d, t.GetPartition(pid), reorg.Job.SnapshotVer, reorg.Job.Priority) + failpoint.Inject("mockUpdateCachedSafePoint", func(val failpoint.Value) { + if val.(bool) { + // 18 is physicalShiftBits. + ts := oracle.GetPhysical(time.Now()) << 18 + s := reorg.d.store.(tikv.Storage) + s.UpdateSPCache(uint64(ts), time.Now()) + time.Sleep(time.Millisecond * 3) + } + }) + currentVer, err := getValidCurrentVersion(reorg.d.store) + if err != nil { + return false, errors.Trace(err) + } + start, end, err := getTableRange(reorg.d, t.GetPartition(pid), currentVer.Ver, reorg.Job.Priority) if err != nil { return false, errors.Trace(err) } diff --git a/ddl/reorg.go b/ddl/reorg.go old mode 100644 new mode 100755 index 716254addbf3d..c88e702fe2640 --- a/ddl/reorg.go +++ b/ddl/reorg.go @@ -340,9 +340,18 @@ func getTableRange(d *ddlCtx, tbl table.PhysicalTable, snapshotVer uint64, prior return } +func getValidCurrentVersion(store kv.Storage) (ver kv.Version, err error) { + ver, err = store.CurrentVersion() + if err != nil { + return ver, errors.Trace(err) + } else if ver.Ver <= 0 { + return ver, errInvalidStoreVer.GenWithStack("invalid storage current version %d", ver.Ver) + } + return ver, nil +} + func getReorgInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table) (*reorgInfo, error) { var ( - err error start int64 end int64 pid int64 @@ -352,12 +361,9 @@ func getReorgInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table) (*re if job.SnapshotVer == 0 { info.first = true // get the current version for reorganization if we don't have - var ver kv.Version - ver, err = d.store.CurrentVersion() + ver, err := getValidCurrentVersion(d.store) if err != nil { return nil, errors.Trace(err) - } else if ver.Ver <= 0 { - return nil, errInvalidStoreVer.GenWithStack("invalid storage current version %d", ver.Ver) } tblInfo := tbl.Meta() pid = tblInfo.ID @@ -384,6 +390,7 @@ func getReorgInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table) (*re // Update info should after data persistent. job.SnapshotVer = ver.Ver } else { + var err error start, end, pid, err = t.GetDDLReorgHandle(job) if err != nil { return nil, errors.Trace(err) @@ -395,7 +402,7 @@ func getReorgInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table) (*re info.EndHandle = end info.PhysicalTableID = pid - return &info, errors.Trace(err) + return &info, nil } func (r *reorgInfo) UpdateReorgMeta(txn kv.Transaction, startHandle, endHandle, physicalTableID int64) error { From 888667d0c044ac26c2b91ad47ef6a839f1c4179d Mon Sep 17 00:00:00 2001 From: Lynn Date: Wed, 18 Dec 2019 19:44:22 +0800 Subject: [PATCH 2/2] ddl: address comments --- ddl/index.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ddl/index.go b/ddl/index.go index a6f93f5a70268..1ae2621234250 100755 --- a/ddl/index.go +++ b/ddl/index.go @@ -1426,7 +1426,7 @@ func (w *worker) updateReorgInfo(t table.PartitionedTable, reorg *reorgInfo) (bo failpoint.Inject("mockUpdateCachedSafePoint", func(val failpoint.Value) { if val.(bool) { - // 18 is physicalShiftBits. + // 18 is for the logical time. ts := oracle.GetPhysical(time.Now()) << 18 s := reorg.d.store.(tikv.Storage) s.UpdateSPCache(uint64(ts), time.Now())