From e404a014219e38a1b129c2b208221e80c1714d88 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 15 Dec 2018 14:52:00 +0800 Subject: [PATCH 01/48] ddl: add admin restore support --- ddl/db_test.go | 33 +++++++++++++++++++ ddl/ddl.go | 1 + ddl/ddl_api.go | 12 +++++++ ddl/ddl_worker.go | 2 ++ ddl/delete_range.go | 39 ++++++++++++++++++++-- ddl/mock.go | 5 +++ ddl/table.go | 37 +++++++++++++++++++++ ddl/table_test.go | 21 ++++++++++++ ddl/util/util.go | 9 ++++-- domain/domain.go | 9 ++++++ executor/admin.go | 63 ++++++++++++++++++++++++++++++++++++ executor/builder.go | 10 ++++++ meta/meta.go | 11 +++++++ planner/core/common_plans.go | 6 ++++ planner/core/planbuilder.go | 2 ++ 15 files changed, 256 insertions(+), 4 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 5b68d7e6168c6..a96b36afd95ad 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1856,3 +1856,36 @@ LOOP: s.dom.DDL().(ddl.DDLForTest).SetHook(originalHook) s.mustExec(c, "drop table t1") } + +func (s *testDBSuite) TestRestoreTable(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("create table t_recover (a int);") + defer ddl.SetEmulatorGCEnable(ddl.GetEmulatorGCStatus()) + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + originGC := ddl.GetEmulatorGCStatus() + defer ddl.SetEmulatorGCEnable(originGC) + ddl.SetEmulatorGCEnable(false) + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + rs, err := tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row := rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID := row.GetInt64(0) + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + + // restore table by none exits job. + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) + c.Assert(err, NotNil) +} diff --git a/ddl/ddl.go b/ddl/ddl.go index 336dc587700ba..0c0dcee070d7c 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -210,6 +210,7 @@ type DDL interface { CreateTable(ctx sessionctx.Context, stmt *ast.CreateTableStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error DropIndex(ctx sessionctx.Context, tableIdent ast.Ident, indexName model.CIStr) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 5389ac0ca2305..41c1155d1359d 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -980,6 +980,18 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) { + job := &model.Job{ + SchemaID: schemaID, + TableID: tbInfo.ID, + Type: model.ActionRestoreTable, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{tbInfo, autoID, dropJobID}, + } + err = d.doDDLJob(ctx, job) + return errors.Trace(err) +} + func checkPartitionByHash(pi *model.PartitionInfo) error { if err := checkAddPartitionTooManyPartitions(pi.Num); err != nil { return errors.Trace(err) diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index b1d1553030e53..21511d1808965 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -506,6 +506,8 @@ func (w *worker) runDDLJob(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, ver, err = onAddTablePartition(t, job) case model.ActionModifyTableCharsetAndCollate: ver, err = onModifyTableCharsetAndCollate(t, job) + case model.ActionRestoreTable: + ver, err = w.onRestoreTable(d, t, job) default: // Invalid job, cancel it. job.State = model.JobStateCancelled diff --git a/ddl/delete_range.go b/ddl/delete_range.go index dfee35efe0ca6..da2cd69003be0 100644 --- a/ddl/delete_range.go +++ b/ddl/delete_range.go @@ -19,6 +19,7 @@ import ( "fmt" "math" "sync" + "sync/atomic" "github.com/pingcap/errors" "github.com/pingcap/parser/model" @@ -38,9 +39,16 @@ const ( delBackLog = 128 ) +// enableEmulatorGC means whether to enable emulator GC. The default is enable. +// In some unit test, we want to stop emulator GC, then wen can set enableEmulatorGC to 0. +var emulatorGCEnable = int32(1) + type delRangeManager interface { // addDelRangeJob add a DDL job into gc_delete_range table. addDelRangeJob(job *model.Job) error + // removeFromGCDeleteRange remove delete table job from gc_delete_range table by jobID and tableID. + // It's use for recover the table that was mistakenly deleted. + removeFromGCDeleteRange(jobID, tableID int64) error start() clear() } @@ -90,6 +98,17 @@ func (dr *delRange) addDelRangeJob(job *model.Job) error { return nil } +// removeFromGCDeleteRange implements delRangeManager interface. +func (dr *delRange) removeFromGCDeleteRange(jobID, tableID int64) error { + ctx, err := dr.sessPool.get() + if err != nil { + return errors.Trace(err) + } + defer dr.sessPool.put(ctx) + err = util.RemoveFromGCDeleteRange(ctx, jobID, tableID) + return errors.Trace(err) +} + // start implements delRangeManager interface. func (dr *delRange) start() { if !dr.storeSupport { @@ -118,11 +137,27 @@ func (dr *delRange) startEmulator() { case <-dr.quitCh: return } - err := dr.doDelRangeWork() - terror.Log(errors.Trace(err)) + if GetEmulatorGCStatus() { + err := dr.doDelRangeWork() + terror.Log(errors.Trace(err)) + } } } +// SetEmulatorGCEnable export for testing. +func SetEmulatorGCEnable(enable bool) { + val := int32(0) + if enable { + val = 1 + } + atomic.StoreInt32(&emulatorGCEnable, val) +} + +// GetEmulatorGCStatus export for testing. +func GetEmulatorGCStatus() bool { + return atomic.LoadInt32(&emulatorGCEnable) == 1 +} + func (dr *delRange) doDelRangeWork() error { ctx, err := dr.sessPool.get() if err != nil { diff --git a/ddl/mock.go b/ddl/mock.go index 4acc92f57688b..aebff5011d07e 100644 --- a/ddl/mock.go +++ b/ddl/mock.go @@ -126,6 +126,11 @@ func (dr *mockDelRange) addDelRangeJob(job *model.Job) error { return nil } +// removeFromGCDeleteRange implements delRangeManager interface. +func (dr *mockDelRange) removeFromGCDeleteRange(jobID, tableID int64) error { + return nil +} + // start implements delRangeManager interface. func (dr *mockDelRange) start() { return diff --git a/ddl/table.go b/ddl/table.go index e597517037b7c..39d096363c287 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -72,6 +72,43 @@ func onCreateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) } } +func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { + schemaID := job.SchemaID + tbInfo := &model.TableInfo{} + var autoID, dropJobID int64 + if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID); err != nil { + // Invalid arguments, cancel this job. + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + + err = checkTableNotExists(t, job, schemaID, tbInfo.Name.L) + if err != nil { + return ver, errors.Trace(err) + } + + ver, err = updateSchemaVersion(t, job) + if err != nil { + return ver, errors.Trace(err) + } + + // Remove dropped table DDL job from gc_delete_range table. + err = w.delRangeManager.removeFromGCDeleteRange(dropJobID, tbInfo.ID) + if err != nil { + return ver, errors.Trace(err) + } + + tbInfo.State = model.StatePublic + tbInfo.UpdateTS = t.StartTS + err = t.CreateTableAndSetAutoID(schemaID, tbInfo, autoID) + if err != nil { + return ver, errors.Trace(err) + } + // Finish this job. + job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) + return ver, nil +} + func onDropTable(t *meta.Meta, job *model.Job) (ver int64, _ error) { schemaID := job.SchemaID tableID := job.TableID diff --git a/ddl/table_test.go b/ddl/table_test.go index 73810ec21daa2..62ceb63bf1047 100644 --- a/ddl/table_test.go +++ b/ddl/table_test.go @@ -137,6 +137,21 @@ func testTruncateTable(c *C, ctx sessionctx.Context, d *ddl, dbInfo *model.DBInf return job } +func testRestoreTable(c *C, ctx sessionctx.Context, d *ddl, dbInfo *model.DBInfo, tblInfo *model.TableInfo, dropJob *model.Job) *model.Job { + job := &model.Job{ + SchemaID: dbInfo.ID, + TableID: tblInfo.ID, + Type: model.ActionRestoreTable, + BinlogInfo: &model.HistoryInfo{}, + Args: []interface{}{tblInfo, 0, dropJob.ID}, + } + err := d.doDDLJob(ctx, job) + c.Assert(err, IsNil) + v := getSchemaVer(c, ctx) + checkHistoryJobArgs(c, ctx, job.ID, &historyJobArgs{ver: v, tbl: tblInfo}) + return job +} + func testCheckTableState(c *C, d *ddl, dbInfo *model.DBInfo, tblInfo *model.TableInfo, state model.SchemaState) { kv.RunInNewTxn(d.store, false, func(txn kv.Transaction) error { t := meta.NewMeta(txn) @@ -224,6 +239,12 @@ func (s *testTableSuite) TestTable(c *C) { job = testDropTable(c, ctx, d, s.dbInfo, tblInfo) testCheckJobDone(c, d, job, false) + testCheckTableState(c, d, s.dbInfo, tblInfo, model.StateNone) + + // for restore table + job = testRestoreTable(c, ctx, d, s.dbInfo, tblInfo, job) + testCheckTableState(c, d, s.dbInfo, tblInfo, model.StatePublic) + testCheckJobDone(c, d, job, true) // for truncate table tblInfo = testTableInfo(c, d, "tt", 3) diff --git a/ddl/util/util.go b/ddl/util/util.go index 9635c66380db2..30b1c3d3d2b28 100644 --- a/ddl/util/util.go +++ b/ddl/util/util.go @@ -108,8 +108,13 @@ func CompleteDeleteRange(ctx sessionctx.Context, dr DelRangeTask) error { return errors.Trace(err) } - sql = fmt.Sprintf(completeDeleteRangeSQL, dr.JobID, dr.ElementID) - _, err = ctx.(sqlexec.SQLExecutor).Execute(context.TODO(), sql) + return RemoveFromGCDeleteRange(ctx, dr.JobID, dr.ElementID) +} + +// RemoveFromGCDeleteRange is exported for ddl pkg to use. +func RemoveFromGCDeleteRange(ctx sessionctx.Context, jobID, elementID int64) error { + sql := fmt.Sprintf(completeDeleteRangeSQL, jobID, elementID) + _, err := ctx.(sqlexec.SQLExecutor).Execute(context.TODO(), sql) return errors.Trace(err) } diff --git a/domain/domain.go b/domain/domain.go index 758e90f247a42..cb733d78fbc99 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -253,6 +253,15 @@ func (do *Domain) GetSnapshotInfoSchema(snapshotTS uint64) (infoschema.InfoSchem return snapHandle.Get(), nil } +// GetSnapshotMeta get a new snapshot meta at startTS. +func (do *Domain) GetSnapshotMeta(startTS uint64) (*meta.Meta, error) { + snapshot, err := do.store.GetSnapshot(kv.NewVersion(startTS)) + if err != nil { + return nil, errors.Trace(err) + } + return meta.NewSnapshotMeta(snapshot), nil +} + // DDL gets DDL from domain. func (do *Domain) DDL() ddl.DDL { return do.ddl diff --git a/executor/admin.go b/executor/admin.go index 13505738f28a9..f51f0e77a21a1 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -15,6 +15,7 @@ package executor import ( "context" + "fmt" "math" "github.com/pingcap/errors" @@ -22,14 +23,17 @@ import ( "github.com/pingcap/parser/model" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/distsql" + "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" + "github.com/pingcap/tidb/meta" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/statistics" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/admin" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/timeutil" @@ -156,6 +160,65 @@ func (e *CheckIndexRangeExec) Close() error { return nil } +// RestoreTableExec represents a recover table executor. +// It is built from "admin restore table by job" statement, +// is used to recover the table that deleted by mistake. +type RestoreTableExec struct { + baseExecutor + jobID int64 +} + +// Open implements the Executor Open interface. +func (e *RestoreTableExec) Open(ctx context.Context) error { + if err := e.baseExecutor.Open(ctx); err != nil { + return errors.Trace(err) + } + return nil +} + +// Next implements the Executor Open interface. +func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { + t := meta.NewMeta(e.ctx.Txn(true)) + job, err := t.GetHistoryDDLJob(e.jobID) + if err != nil { + return errors.Trace(err) + } + if job == nil { + return admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) + } + if job.Type != model.ActionDropTable { + return errors.Errorf("Job %v doesn't drop any table", job.ID) + } + + dom := domain.GetDomain(e.ctx) + // Get the snapshot infoSchema before drop table. + snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) + if err != nil { + return errors.Trace(err) + } + // Get table meta from snapshot infoSchema. + table, ok := snapInfo.TableByID(job.TableID) + if !ok { + return infoschema.ErrTableNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", job.SchemaID), + fmt.Sprintf("(Table ID %d)", job.TableID), + ) + } + // Get table original autoID before table drop. + m, err := dom.GetSnapshotMeta(job.StartTS) + if err != nil { + return errors.Trace(err) + } + autoID, err := m.GetAutoTableID(job.SchemaID, job.TableID) + if err != nil { + return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) + } + // Call DDL RestoreTable + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID) + return errors.Trace(err) + +} + // RecoverIndexExec represents a recover index executor. // It is built from "admin recover index" statement, is used to backfill // corrupted index. diff --git a/executor/builder.go b/executor/builder.go index db78a599d49d9..1ebadaef53a30 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -84,6 +84,8 @@ func (b *executorBuilder) build(p plannercore.Plan) Executor { return b.buildCheckIndexRange(v) case *plannercore.ChecksumTable: return b.buildChecksumTable(v) + case *plannercore.RestoreTable: + return b.buildRestoreTable(v) case *plannercore.DDL: return b.buildDDL(v) case *plannercore.Deallocate: @@ -325,6 +327,14 @@ func (b *executorBuilder) buildRecoverIndex(v *plannercore.RecoverIndex) Executo return e } +func (b *executorBuilder) buildRestoreTable(v *plannercore.RestoreTable) Executor { + e := &RestoreTableExec{ + baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ExplainID()), + jobID: v.JobID, + } + return e +} + func buildCleanupIndexCols(tblInfo *model.TableInfo, indexInfo *model.IndexInfo) []*model.ColumnInfo { columns := make([]*model.ColumnInfo, 0, len(indexInfo.Columns)+1) for _, idxCol := range indexInfo.Columns { diff --git a/meta/meta.go b/meta/meta.go index 901b69f450adc..d186f89147eb1 100644 --- a/meta/meta.go +++ b/meta/meta.go @@ -296,6 +296,17 @@ func (m *Meta) CreateTable(dbID int64, tableInfo *model.TableInfo) error { return m.txn.HSet(dbKey, tableKey, data) } +// CreateTableAndSetAutoID creates a table with tableInfo in database, +// and rebase the table autoID. +func (m *Meta) CreateTableAndSetAutoID(dbID int64, tableInfo *model.TableInfo, autoID int64) error { + err := m.CreateTable(dbID, tableInfo) + if err != nil { + return errors.Trace(err) + } + _, err = m.txn.HInc(m.dbKey(dbID), m.autoTableIDKey(tableInfo.ID), autoID) + return errors.Trace(err) +} + // DropDatabase drops whole database. func (m *Meta) DropDatabase(dbID int64) error { // Check if db exists. diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index 217c4449dafc0..58c9973bf9c0e 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -84,6 +84,12 @@ type RecoverIndex struct { IndexName string } +// RestoreTable is used for recover deleted files by mistake. +type RestoreTable struct { + baseSchemaProducer + JobID int64 +} + // CleanupIndex is used to delete dangling index data. type CleanupIndex struct { baseSchemaProducer diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 6faad83705589..ad60d24f9e0f0 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -533,6 +533,8 @@ func (b *PlanBuilder) buildAdmin(as *ast.AdminStmt) (Plan, error) { p := &ShowSlow{ShowSlow: as.ShowSlow} p.SetSchema(buildShowSlowSchema()) ret = p + case ast.AdminRestoreTable: + ret = &RestoreTable{JobID: as.JobIDs[0]} default: return nil, ErrUnsupportedType.GenWithStack("Unsupported ast.AdminStmt(%T) for buildAdmin", as) } From 6991fa18cf5c268df4c96acea85ed8068cd02ac6 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 15 Dec 2018 18:34:38 +0800 Subject: [PATCH 02/48] check gc enable status --- ddl/db_test.go | 2 ++ ddl/ddl.go | 2 +- ddl/ddl_api.go | 4 ++-- ddl/table.go | 41 ++++++++++++++++++++++++++++++++++++++++- ddl/table_test.go | 21 --------------------- executor/admin.go | 22 +++++++++++++++++++++- util/admin/admin.go | 33 +++++++++++++++++++++++++++++++++ 7 files changed, 99 insertions(+), 26 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index a96b36afd95ad..ece5def6f7c57 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1878,6 +1878,8 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(row.GetString(1), Equals, "test_restore") c.Assert(row.GetString(3), Equals, "drop table") jobID := row.GetInt64(0) + // enable GC first. + err = admin.EnableGCAfterRecover(tk.Se) tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) // check recover table meta and data record. tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) diff --git a/ddl/ddl.go b/ddl/ddl.go index 0c0dcee070d7c..c20ae1cfd96d3 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -210,7 +210,7 @@ type DDL interface { CreateTable(ctx sessionctx.Context, stmt *ast.CreateTableStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) - RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error DropIndex(ctx sessionctx.Context, tableIdent ast.Ident, indexName model.CIStr) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 41c1155d1359d..71805a3db9983 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -980,13 +980,13 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } -func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) { +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) { job := &model.Job{ SchemaID: schemaID, TableID: tbInfo.ID, Type: model.ActionRestoreTable, BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tbInfo, autoID, dropJobID}, + Args: []interface{}{tbInfo, autoID, dropJobID, enableGCAfterRecover}, } err = d.doDDLJob(ctx, job) return errors.Trace(err) diff --git a/ddl/table.go b/ddl/table.go index 39d096363c287..3c1adcc9ec48d 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -27,6 +27,7 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/tablecodec" + "github.com/pingcap/tidb/util/admin" log "github.com/sirupsen/logrus" ) @@ -73,10 +74,22 @@ func onCreateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) } func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { + // check gc enable status again. + gcEnable, isNull, err := checkGCEnable(w) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + if isNull || gcEnable { + job.State = model.JobStateCancelled + return ver, errors.Errorf("can not found gc enable variable in mysql.tidb") + } + schemaID := job.SchemaID tbInfo := &model.TableInfo{} var autoID, dropJobID int64 - if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID); err != nil { + var enableGCAfterRecover bool + if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover); err != nil { // Invalid arguments, cancel this job. job.State = model.JobStateCancelled return ver, errors.Trace(err) @@ -104,6 +117,12 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in if err != nil { return ver, errors.Trace(err) } + if enableGCAfterRecover { + err = enableGC(w) + if err != nil { + return ver, errors.Trace(err) + } + } // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) return ver, nil @@ -166,6 +185,26 @@ func onDropTable(t *meta.Meta, job *model.Job) (ver int64, _ error) { return ver, errors.Trace(err) } +func enableGC(w *worker) error { + ctx, err := w.sessPool.get() + if err != nil { + return errors.Trace(err) + } + defer w.sessPool.put(ctx) + + return admin.EnableGCAfterRecover(ctx) +} + +func checkGCEnable(w *worker) (enable bool, isNull bool, err error) { + ctx, err := w.sessPool.get() + if err != nil { + return false, false, errors.Trace(err) + } + defer w.sessPool.put(ctx) + + return admin.CheckGCEnableStatus(ctx) +} + type splitableStore interface { SplitRegion(splitKey kv.Key) error } diff --git a/ddl/table_test.go b/ddl/table_test.go index 62ceb63bf1047..73810ec21daa2 100644 --- a/ddl/table_test.go +++ b/ddl/table_test.go @@ -137,21 +137,6 @@ func testTruncateTable(c *C, ctx sessionctx.Context, d *ddl, dbInfo *model.DBInf return job } -func testRestoreTable(c *C, ctx sessionctx.Context, d *ddl, dbInfo *model.DBInfo, tblInfo *model.TableInfo, dropJob *model.Job) *model.Job { - job := &model.Job{ - SchemaID: dbInfo.ID, - TableID: tblInfo.ID, - Type: model.ActionRestoreTable, - BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tblInfo, 0, dropJob.ID}, - } - err := d.doDDLJob(ctx, job) - c.Assert(err, IsNil) - v := getSchemaVer(c, ctx) - checkHistoryJobArgs(c, ctx, job.ID, &historyJobArgs{ver: v, tbl: tblInfo}) - return job -} - func testCheckTableState(c *C, d *ddl, dbInfo *model.DBInfo, tblInfo *model.TableInfo, state model.SchemaState) { kv.RunInNewTxn(d.store, false, func(txn kv.Transaction) error { t := meta.NewMeta(txn) @@ -239,12 +224,6 @@ func (s *testTableSuite) TestTable(c *C) { job = testDropTable(c, ctx, d, s.dbInfo, tblInfo) testCheckJobDone(c, d, job, false) - testCheckTableState(c, d, s.dbInfo, tblInfo, model.StateNone) - - // for restore table - job = testRestoreTable(c, ctx, d, s.dbInfo, tblInfo, job) - testCheckTableState(c, d, s.dbInfo, tblInfo, model.StatePublic) - testCheckJobDone(c, d, job, true) // for truncate table tblInfo = testTableInfo(c, d, "tt", 3) diff --git a/executor/admin.go b/executor/admin.go index f51f0e77a21a1..ed708bd87487d 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -178,6 +178,26 @@ func (e *RestoreTableExec) Open(ctx context.Context) error { // Next implements the Executor Open interface. func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { + enableGCAfterRecover, isNull, err := admin.CheckGCEnableStatus(e.ctx) + if err != nil { + return err + } + if isNull { + return errors.Errorf("can not found gc enable variable in mysql.tidb") + } + if enableGCAfterRecover { + err = admin.DisableGCForRecover(e.ctx) + if err != nil { + return err + } + defer func() { + // if err == nil, should be enable gc in ddl owner. + if err != nil { + admin.EnableGCAfterRecover(e.ctx) + } + }() + } + t := meta.NewMeta(e.ctx.Txn(true)) job, err := t.GetHistoryDDLJob(e.jobID) if err != nil { @@ -214,7 +234,7 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID) + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, enableGCAfterRecover) return errors.Trace(err) } diff --git a/util/admin/admin.go b/util/admin/admin.go index a09cfce31982c..1f99fddf78c44 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -711,6 +711,39 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab return nil } +// CheckGCEnableStatus is +func CheckGCEnableStatus(ctx sessionctx.Context) (enable bool, isNull bool, err error) { + sql := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, "tikv_gc_enable") + rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + if err != nil { + return false, false, errors.Trace(err) + } + if len(rows) != 1 { + return false, true, nil + } + return rows[0].GetString(0) == "true", false, nil +} + +// DisableGCForRecover is +func DisableGCForRecover(ctx sessionctx.Context) error { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') + ON DUPLICATE KEY + UPDATE variable_value = '%[2]s', comment = '%[3]s'`, + "tikv_gc_enable", "false", "Current GC enable status") + _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + return errors.Trace(err) +} + +// EnableGCAfterRecover is +func EnableGCAfterRecover(ctx sessionctx.Context) error { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') + ON DUPLICATE KEY + UPDATE variable_value = '%[2]s', comment = '%[3]s'`, + "tikv_gc_enable", "true", "Current GC enable status") + _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + return errors.Trace(err) +} + // admin error codes. const ( codeDataNotEqual terror.ErrCode = 1 From 72b921dac2e48ae2111f1715baba50352477304f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 15 Dec 2018 21:09:27 +0800 Subject: [PATCH 03/48] check gc safe point --- ddl/db_test.go | 35 ++++++++++++++++++++++++++++++++++- ddl/table.go | 8 ++++---- executor/admin.go | 11 +++++++---- executor/set.go | 1 + util/admin/admin.go | 8 ++++---- 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index ece5def6f7c57..7727e7a39f92c 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -37,6 +37,7 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/mockstore/mocktikv" "github.com/pingcap/tidb/table" @@ -1863,13 +1864,24 @@ func (s *testDBSuite) TestRestoreTable(c *C) { tk.MustExec("use test_restore") tk.MustExec("create table t_recover (a int);") defer ddl.SetEmulatorGCEnable(ddl.GetEmulatorGCStatus()) + // disable emulator GC. // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. originGC := ddl.GetEmulatorGCStatus() defer ddl.SetEmulatorGCEnable(originGC) ddl.SetEmulatorGCEnable(false) + gcTimeFormat := "20060102-15:04:05 -0700 MST" + + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) + + safePointSql := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + tk.MustExec("insert into t_recover values (1),(2),(3)") tk.MustExec("drop table t_recover") + rs, err := tk.Exec("admin show ddl jobs") c.Assert(err, IsNil) rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) @@ -1878,9 +1890,30 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(row.GetString(1), Equals, "test_restore") c.Assert(row.GetString(3), Equals, "drop table") jobID := row.GetInt64(0) - // enable GC first. + + // if gc enable is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_enable'") + err = admin.EnableGCAfterRecover(tk.Se) + c.Assert(err, IsNil) + + // if gc safe point is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + + // recover job is before gc safe point + tk.MustExec(fmt.Sprintf(safePointSql, timeAfterDrop)) + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) + + // recover job after gc safe point + tk.MustExec(fmt.Sprintf(safePointSql, timeBeforeDrop)) tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + // check recover table meta and data record. tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) // check recover table autoID. diff --git a/ddl/table.go b/ddl/table.go index 3c1adcc9ec48d..d8f2a4b617b89 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -75,12 +75,12 @@ func onCreateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ error) func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { // check gc enable status again. - gcEnable, isNull, err := checkGCEnable(w) + gcEnable, err := checkGCEnable(w) if err != nil { job.State = model.JobStateCancelled return ver, errors.Trace(err) } - if isNull || gcEnable { + if gcEnable { job.State = model.JobStateCancelled return ver, errors.Errorf("can not found gc enable variable in mysql.tidb") } @@ -195,10 +195,10 @@ func enableGC(w *worker) error { return admin.EnableGCAfterRecover(ctx) } -func checkGCEnable(w *worker) (enable bool, isNull bool, err error) { +func checkGCEnable(w *worker) (enable bool, err error) { ctx, err := w.sessPool.get() if err != nil { - return false, false, errors.Trace(err) + return false, errors.Trace(err) } defer w.sessPool.put(ctx) diff --git a/executor/admin.go b/executor/admin.go index ed708bd87487d..622f3e11cb01c 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -178,13 +178,10 @@ func (e *RestoreTableExec) Open(ctx context.Context) error { // Next implements the Executor Open interface. func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { - enableGCAfterRecover, isNull, err := admin.CheckGCEnableStatus(e.ctx) + enableGCAfterRecover, err := admin.CheckGCEnableStatus(e.ctx) if err != nil { return err } - if isNull { - return errors.Errorf("can not found gc enable variable in mysql.tidb") - } if enableGCAfterRecover { err = admin.DisableGCForRecover(e.ctx) if err != nil { @@ -210,6 +207,12 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("Job %v doesn't drop any table", job.ID) } + // check gc safe point + err = validateSnapshot(e.ctx, job.StartTS) + if err != nil { + return errors.Trace(err) + } + dom := domain.GetDomain(e.ctx) // Get the snapshot infoSchema before drop table. snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) diff --git a/executor/set.go b/executor/set.go index 84e329792b1ed..26bca0cd6ee9a 100644 --- a/executor/set.go +++ b/executor/set.go @@ -200,6 +200,7 @@ func validateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { return errors.Trace(err) } safePointTS := variable.GoTimeToTS(safePointTime) + fmt.Printf("\n\nvalid safe point: %v\nformat: %v\nsafeTS: %v\nsnapTs: %v-------------\n", safePointTime, safePointTime.Format(gcTimeFormat), safePointTS, snapshotTS) if safePointTS > snapshotTS { return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) } diff --git a/util/admin/admin.go b/util/admin/admin.go index 1f99fddf78c44..913e55073dcd0 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -712,16 +712,16 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab } // CheckGCEnableStatus is -func CheckGCEnableStatus(ctx sessionctx.Context) (enable bool, isNull bool, err error) { +func CheckGCEnableStatus(ctx sessionctx.Context) (enable bool, err error) { sql := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, "tikv_gc_enable") rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) if err != nil { - return false, false, errors.Trace(err) + return false, errors.Trace(err) } if len(rows) != 1 { - return false, true, nil + return false, errors.New("can not get 'tikv_gc_enable'") } - return rows[0].GetString(0) == "true", false, nil + return rows[0].GetString(0) == "true", nil } // DisableGCForRecover is From fd70afa4f444c0f64f262004fb5df696c032846a Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 18 Dec 2018 21:01:52 +0800 Subject: [PATCH 04/48] change go.mod temporarily --- go.mod | 2 ++ go.sum | 2 ++ 2 files changed, 4 insertions(+) diff --git a/go.mod b/go.mod index da2ef76738f7c..f935f80fc6f2d 100644 --- a/go.mod +++ b/go.mod @@ -85,3 +85,5 @@ require ( sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4 sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) + +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 diff --git a/go.sum b/go.sum index a777b280a1f47..a7e036b2f526b 100644 --- a/go.sum +++ b/go.sum @@ -22,6 +22,8 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= +github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 h1:BK94jexUhMnI0t3KLRV0qID818CgXGRrFsRktdoKVdg= +github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= From a2169acf9f46002cf98d3d7c62558c006786aed2 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 18 Dec 2018 21:28:11 +0800 Subject: [PATCH 05/48] refine err msg --- ddl/table.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ddl/table.go b/ddl/table.go index f63086f2146a5..33e8740f63ffb 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -119,7 +119,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in } if gcEnable { job.State = model.JobStateCancelled - return ver, errors.Errorf("can not found gc enable variable in mysql.tidb") + return ver, errors.Errorf("can not restore deleted table when gc is enable") } schemaID := job.SchemaID @@ -631,4 +631,3 @@ func checkAddPartitionValue(meta *model.TableInfo, part *model.PartitionInfo) er } return nil } - From b6596179f6aa83b799b2b9ce0958a1cec896ad3e Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 24 Dec 2018 12:50:32 +0800 Subject: [PATCH 06/48] fix txn --- executor/admin.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/executor/admin.go b/executor/admin.go index 82d32d47ac3ab..fcbc9fa17036e 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -201,8 +201,11 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { } }() } - - t := meta.NewMeta(e.ctx.Txn(true)) + txn, err := e.ctx.Txn(true) + if err != nil { + return errors.Trace(err) + } + t := meta.NewMeta(txn) job, err := t.GetHistoryDDLJob(e.jobID) if err != nil { return errors.Trace(err) From 83477fb47058bb0a199838ae84a34b5a6361a977 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 24 Dec 2018 13:03:13 +0800 Subject: [PATCH 07/48] fix go.mod --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 6ae4f855b62f8..c7a13afb6cdb8 100644 --- a/go.mod +++ b/go.mod @@ -87,4 +87,4 @@ require ( sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262 diff --git a/go.sum b/go.sum index 88546dc3b0f66..4046cfc0ed265 100644 --- a/go.sum +++ b/go.sum @@ -25,6 +25,8 @@ github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbp github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 h1:BK94jexUhMnI0t3KLRV0qID818CgXGRrFsRktdoKVdg= github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= +github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262 h1:A5MeD7YBqYOEq//Nt4zqr4AL4ks5wxcOqUJoEwZbpMQ= +github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= From ee5f7e0ee226fbe0873ad594c440c449e8b64ba0 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 24 Dec 2018 13:12:51 +0800 Subject: [PATCH 08/48] update go.mod parser --- go.mod | 2 +- go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c7a13afb6cdb8..ae223519cf974 100644 --- a/go.mod +++ b/go.mod @@ -87,4 +87,4 @@ require ( sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262 +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33 diff --git a/go.sum b/go.sum index 4046cfc0ed265..69365ee6726c0 100644 --- a/go.sum +++ b/go.sum @@ -27,6 +27,8 @@ github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 h1:BK94jexUhMnI0 github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262 h1:A5MeD7YBqYOEq//Nt4zqr4AL4ks5wxcOqUJoEwZbpMQ= github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= +github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33 h1:ZVfWmZV+/Z71FZ6R7ywSS9id915nc5lzGVbWG48WptI= +github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= From 7669086a1eaee36f015f30845f7903d7ac16a0b4 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 24 Dec 2018 13:23:48 +0800 Subject: [PATCH 09/48] fix lint --- ddl/db_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index ac989df708e69..8bac1bae36dda 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -1881,7 +1881,7 @@ func (s *testDBSuite) TestRestoreTable(c *C) { timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) - safePointSql := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') ON DUPLICATE KEY UPDATE variable_value = '%[1]s'` @@ -1911,13 +1911,13 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") // recover job is before gc safe point - tk.MustExec(fmt.Sprintf(safePointSql, timeAfterDrop)) + tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) // recover job after gc safe point - tk.MustExec(fmt.Sprintf(safePointSql, timeBeforeDrop)) + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) // check recover table meta and data record. From 8278a9d1078c8ba2ee55758d94119a83f9a68375 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 24 Dec 2018 13:29:18 +0800 Subject: [PATCH 10/48] fix make tidy --- go.sum | 5 ----- 1 file changed, 5 deletions(-) diff --git a/go.sum b/go.sum index 69365ee6726c0..4758ada384e01 100644 --- a/go.sum +++ b/go.sum @@ -23,11 +23,6 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= -github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223 h1:BK94jexUhMnI0t3KLRV0qID818CgXGRrFsRktdoKVdg= -github.com/crazycs520/parser v0.0.0-20181218112110-2e87cf0bc223/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= -github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262 h1:A5MeD7YBqYOEq//Nt4zqr4AL4ks5wxcOqUJoEwZbpMQ= -github.com/crazycs520/parser v0.0.0-20181224045756-61735cdb2262/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= -github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33 h1:ZVfWmZV+/Z71FZ6R7ywSS9id915nc5lzGVbWG48WptI= github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= From a0945d6f94fdf4ee8abd428a47d5c6792acec3c2 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 12:15:03 +0800 Subject: [PATCH 11/48] update go.mod parser --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index ae223519cf974..d30253f02c61a 100644 --- a/go.mod +++ b/go.mod @@ -87,4 +87,4 @@ require ( sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33 +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0 diff --git a/go.sum b/go.sum index 4758ada384e01..f1f618b5da07e 100644 --- a/go.sum +++ b/go.sum @@ -24,6 +24,8 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7 github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= +github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0 h1:DV34+vatddXlp6pUnJG3NYySR9wlHCgzbHB19brA380= +github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= @@ -128,8 +130,6 @@ github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e h1:P73/4dPCL96rG github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c h1:Qf5St5XGwKgKQLar9lEXoeO0hJMVaFBj3JqvFguWtVg= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c/go.mod h1:Ja9XPjot9q4/3JyCZodnWDGNXt4pKemhIYCvVJM7P24= -github.com/pingcap/parser v0.0.0-20181221050948-947d4ab3052f h1:X2ZRYBERoJ5VDp7CdtXWfwbKqbeYn2kkdGA0b5/VwJ8= -github.com/pingcap/parser v0.0.0-20181221050948-947d4ab3052f/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA= github.com/pingcap/pd v2.1.0-rc.4+incompatible h1:/buwGk04aHO5odk/+O8ZOXGs4qkUjYTJ2UpCJXna8NE= github.com/pingcap/pd v2.1.0-rc.4+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E= github.com/pingcap/tidb-tools v2.1.1-0.20181218072513-b2235d442b06+incompatible h1:Bsd+NHosPVowEGB3BCx+2d8wUQGDTXSSC5ljeNS6cXo= From f829fe34d3ccbcf8c3f6839585b61e6ea28e2a64 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 12:51:54 +0800 Subject: [PATCH 12/48] update go.mod parser --- go.mod | 4 +--- go.sum | 5 ++--- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 70803c4a48370..ce9d2979b270d 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3 github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c - github.com/pingcap/parser v0.0.0-20190103075927-c065c7404641 + github.com/pingcap/parser v0.0.0-20190104042418-24cab7ea7628 github.com/pingcap/pd v2.1.0-rc.4+incompatible github.com/pingcap/tidb-tools v2.1.1-0.20181218072513-b2235d442b06+incompatible github.com/pingcap/tipb v0.0.0-20181012112600-11e33c750323 @@ -86,5 +86,3 @@ require ( sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4 sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) - -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0 diff --git a/go.sum b/go.sum index 98c28b0e8ea15..d9a4c662fcacd 100644 --- a/go.sum +++ b/go.sum @@ -22,9 +22,6 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= -github.com/crazycs520/parser v0.0.0-20181224051105-ca9babd29a33/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= -github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0 h1:DV34+vatddXlp6pUnJG3NYySR9wlHCgzbHB19brA380= -github.com/crazycs520/parser v0.0.0-20190104035953-e777b29548c0/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= @@ -113,6 +110,8 @@ github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e h1:P73/4dPCL96rG github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c h1:Qf5St5XGwKgKQLar9lEXoeO0hJMVaFBj3JqvFguWtVg= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c/go.mod h1:Ja9XPjot9q4/3JyCZodnWDGNXt4pKemhIYCvVJM7P24= +github.com/pingcap/parser v0.0.0-20190104042418-24cab7ea7628 h1:lOAOKv3lUTV2wRYOhbH4Cu2AW9juFUbIIAksFN7SUgk= +github.com/pingcap/parser v0.0.0-20190104042418-24cab7ea7628/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA= github.com/pingcap/pd v2.1.0-rc.4+incompatible h1:/buwGk04aHO5odk/+O8ZOXGs4qkUjYTJ2UpCJXna8NE= github.com/pingcap/pd v2.1.0-rc.4+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E= github.com/pingcap/tidb-tools v2.1.1-0.20181218072513-b2235d442b06+incompatible h1:Bsd+NHosPVowEGB3BCx+2d8wUQGDTXSSC5ljeNS6cXo= From 7e943f6b62068dbc31da79d6e291f3089f37b9c1 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 13:00:08 +0800 Subject: [PATCH 13/48] fix error check --- executor/admin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/admin.go b/executor/admin.go index fcbc9fa17036e..dec98d27d1a73 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -197,7 +197,7 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { defer func() { // if err == nil, should be enable gc in ddl owner. if err != nil { - admin.EnableGCAfterRecover(e.ctx) + log.Error(admin.EnableGCAfterRecover(e.ctx)) } }() } From 1182242c71cd625920253360a6ce3b9a4ed16ee7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 4 Jan 2019 13:10:08 +0800 Subject: [PATCH 14/48] refine code --- ddl/ddl.go | 2 +- ddl/ddl_api.go | 4 ++-- ddl/table.go | 9 +-------- executor/admin.go | 11 ++++------- 4 files changed, 8 insertions(+), 18 deletions(-) diff --git a/ddl/ddl.go b/ddl/ddl.go index 0d501fdbd2dab..0029988de16ae 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -215,7 +215,7 @@ type DDL interface { CreateView(ctx sessionctx.Context, stmt *ast.CreateViewStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) - RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) DropView(ctx sessionctx.Context, tableIdent ast.Ident) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index d5043a59b61ae..5491801e35c6e 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1024,13 +1024,13 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } -func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) { +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) { job := &model.Job{ SchemaID: schemaID, TableID: tbInfo.ID, Type: model.ActionRestoreTable, BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tbInfo, autoID, dropJobID, enableGCAfterRecover}, + Args: []interface{}{tbInfo, autoID, dropJobID}, } err = d.doDDLJob(ctx, job) return errors.Trace(err) diff --git a/ddl/table.go b/ddl/table.go index a5cf291b72f61..695c4288f6ea7 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -163,8 +163,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in schemaID := job.SchemaID tbInfo := &model.TableInfo{} var autoID, dropJobID int64 - var enableGCAfterRecover bool - if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover); err != nil { + if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID); err != nil { // Invalid arguments, cancel this job. job.State = model.JobStateCancelled return ver, errors.Trace(err) @@ -192,12 +191,6 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in if err != nil { return ver, errors.Trace(err) } - if enableGCAfterRecover { - err = enableGC(w) - if err != nil { - return ver, errors.Trace(err) - } - } // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) return ver, nil diff --git a/executor/admin.go b/executor/admin.go index dec98d27d1a73..f470ad76d548e 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -185,20 +185,17 @@ func (e *RestoreTableExec) Open(ctx context.Context) error { // Next implements the Executor Open interface. func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { - enableGCAfterRecover, err := admin.CheckGCEnableStatus(e.ctx) + gcEnable, err := admin.CheckGCEnableStatus(e.ctx) if err != nil { return err } - if enableGCAfterRecover { + if gcEnable { err = admin.DisableGCForRecover(e.ctx) if err != nil { return err } defer func() { - // if err == nil, should be enable gc in ddl owner. - if err != nil { - log.Error(admin.EnableGCAfterRecover(e.ctx)) - } + log.Error(admin.EnableGCAfterRecover(e.ctx)) }() } txn, err := e.ctx.Txn(true) @@ -247,7 +244,7 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, enableGCAfterRecover) + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID) return errors.Trace(err) } From 864c006631c4f7161d56061bcd2d1c3c2ffa06f9 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 7 Jan 2019 12:40:38 +0800 Subject: [PATCH 15/48] address comment --- ddl/ddl.go | 2 +- ddl/ddl_api.go | 4 ++-- ddl/table.go | 9 ++++++++- executor/admin.go | 7 +++++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/ddl/ddl.go b/ddl/ddl.go index 0029988de16ae..0d501fdbd2dab 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -215,7 +215,7 @@ type DDL interface { CreateView(ctx sessionctx.Context, stmt *ast.CreateViewStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) - RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) DropView(ctx sessionctx.Context, tableIdent ast.Ident) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index f3b4d4054a5c6..579f47e9835b5 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1023,13 +1023,13 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } -func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64) (err error) { +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) { job := &model.Job{ SchemaID: schemaID, TableID: tbInfo.ID, Type: model.ActionRestoreTable, BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tbInfo, autoID, dropJobID}, + Args: []interface{}{tbInfo, autoID, dropJobID, enableGCAfterRecover}, } err = d.doDDLJob(ctx, job) return errors.Trace(err) diff --git a/ddl/table.go b/ddl/table.go index 695c4288f6ea7..a5cf291b72f61 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -163,7 +163,8 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in schemaID := job.SchemaID tbInfo := &model.TableInfo{} var autoID, dropJobID int64 - if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID); err != nil { + var enableGCAfterRecover bool + if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover); err != nil { // Invalid arguments, cancel this job. job.State = model.JobStateCancelled return ver, errors.Trace(err) @@ -191,6 +192,12 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in if err != nil { return ver, errors.Trace(err) } + if enableGCAfterRecover { + err = enableGC(w) + if err != nil { + return ver, errors.Trace(err) + } + } // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) return ver, nil diff --git a/executor/admin.go b/executor/admin.go index f470ad76d548e..fa85ed6cab151 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -195,7 +195,10 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return err } defer func() { - log.Error(admin.EnableGCAfterRecover(e.ctx)) + // if err == nil, should be enable gc in ddl owner. + if err != nil { + log.Error(admin.EnableGCAfterRecover(e.ctx)) + } }() } txn, err := e.ctx.Txn(true) @@ -244,7 +247,7 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID) + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, gcEnable) return errors.Trace(err) } From ff4c4706b93a2779fc2f4e9903f46038f9a2fc48 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 7 Jan 2019 12:51:53 +0800 Subject: [PATCH 16/48] add test --- ddl/db_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/ddl/db_test.go b/ddl/db_test.go index 66c67127debd5..68902c00cef85 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2101,6 +2101,34 @@ func (s *testDBSuite) TestRestoreTable(c *C) { // restore table by none exits job. _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) c.Assert(err, NotNil) + + // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + err = admin.DisableGCForRecover(tk.Se) + c.Assert(err, IsNil) + + tk.MustExec("delete from t_recover where a > 1") + tk.MustExec("drop table t_recover") + rs, err = tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err = session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row = rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID = row.GetInt64(0) + + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (7),(8),(9)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) + + gcEnable, err := admin.CheckGCEnableStatus(tk.Se) + c.Assert(err, IsNil) + c.Assert(gcEnable, Equals, false) + } func (s *testDBSuite) TestTransactionOnAddDropColumn(c *C) { From 01cb0d86a726e6b0c0d810f2e733d145432ef878 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 7 Jan 2019 12:54:24 +0800 Subject: [PATCH 17/48] address comment --- ddl/db_test.go | 4 ++-- ddl/delete_range.go | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 68902c00cef85..0c8523a9c5b44 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2041,11 +2041,11 @@ func (s *testDBSuite) TestRestoreTable(c *C) { tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") tk.MustExec("create table t_recover (a int);") - defer ddl.SetEmulatorGCEnable(ddl.GetEmulatorGCStatus()) + defer ddl.SetEmulatorGCEnable(ddl.EmulatorGCEnable()) // disable emulator GC. // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. - originGC := ddl.GetEmulatorGCStatus() + originGC := ddl.EmulatorGCEnable() defer ddl.SetEmulatorGCEnable(originGC) ddl.SetEmulatorGCEnable(false) gcTimeFormat := "20060102-15:04:05 -0700 MST" diff --git a/ddl/delete_range.go b/ddl/delete_range.go index 42b7385a82166..c072feddc8898 100644 --- a/ddl/delete_range.go +++ b/ddl/delete_range.go @@ -136,7 +136,7 @@ func (dr *delRange) startEmulator() { case <-dr.quitCh: return } - if GetEmulatorGCStatus() { + if EmulatorGCEnable() { err := dr.doDelRangeWork() terror.Log(errors.Trace(err)) } @@ -152,8 +152,8 @@ func SetEmulatorGCEnable(enable bool) { atomic.StoreInt32(&emulatorGCEnable, val) } -// GetEmulatorGCStatus export for testing. -func GetEmulatorGCStatus() bool { +// EmulatorGCEnable export for testing. +func EmulatorGCEnable() bool { return atomic.LoadInt32(&emulatorGCEnable) == 1 } From c7ed2dc79570a0080a8e9df536281d86a173bd34 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 19:55:36 +0800 Subject: [PATCH 18/48] address comment --- ddl/db_test.go | 12 ++++++++---- ddl/delete_range.go | 21 +++++++++++---------- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index c0246ac1eae7f..eaf47b857234e 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2134,13 +2134,17 @@ func (s *testDBSuite) TestRestoreTable(c *C) { tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") tk.MustExec("create table t_recover (a int);") - defer ddl.SetEmulatorGCEnable(ddl.EmulatorGCEnable()) + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) // disable emulator GC. // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. - originGC := ddl.EmulatorGCEnable() - defer ddl.SetEmulatorGCEnable(originGC) - ddl.SetEmulatorGCEnable(false) + ddl.EmulatorGCDisable() gcTimeFormat := "20060102-15:04:05 -0700 MST" timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) diff --git a/ddl/delete_range.go b/ddl/delete_range.go index c072feddc8898..4ddab17ad6dd2 100644 --- a/ddl/delete_range.go +++ b/ddl/delete_range.go @@ -136,24 +136,25 @@ func (dr *delRange) startEmulator() { case <-dr.quitCh: return } - if EmulatorGCEnable() { + if IsEmulatorGCEnable() { err := dr.doDelRangeWork() terror.Log(errors.Trace(err)) } } } -// SetEmulatorGCEnable export for testing. -func SetEmulatorGCEnable(enable bool) { - val := int32(0) - if enable { - val = 1 - } - atomic.StoreInt32(&emulatorGCEnable, val) +// EmulatorGCEnable enables emulator gc. It exports for testing. +func EmulatorGCEnable() { + atomic.StoreInt32(&emulatorGCEnable, 1) +} + +// EmulatorGCDisable disables emulator gc. It exports for testing. +func EmulatorGCDisable() { + atomic.StoreInt32(&emulatorGCEnable, 0) } -// EmulatorGCEnable export for testing. -func EmulatorGCEnable() bool { +// IsEmulatorGCEnable indicates whether emulator gc enabled. It exports for testing. +func IsEmulatorGCEnable() bool { return atomic.LoadInt32(&emulatorGCEnable) == 1 } From 31082b04975350ee75876ef76e3e5e0e3bb85fde Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 20:20:40 +0800 Subject: [PATCH 19/48] add gofail test --- ddl/ddl_worker.go | 19 ++++++++++++++ ddl/serial_test.go | 65 ++++++++++++++++++++++++++++++++++++++++++++++ ddl/table.go | 7 +---- store/tikv/txn.go | 10 +++++++ 4 files changed, 95 insertions(+), 6 deletions(-) diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index 4005ffbdc7f72..5ff746529ee87 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -276,6 +276,8 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { err = w.deleteRange(job) case model.ActionDropSchema, model.ActionDropTable, model.ActionTruncateTable, model.ActionDropIndex, model.ActionDropTablePartition, model.ActionTruncateTablePartition: err = w.deleteRange(job) + case model.ActionRestoreTable: + err = finishRestoreTable(w, t, job) } if err != nil { return errors.Trace(err) @@ -293,6 +295,23 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { return errors.Trace(err) } +func finishRestoreTable(w *worker, t *meta.Meta, job *model.Job) error { + tbInfo := &model.TableInfo{} + var autoID, dropJobID int64 + var enableGCAfterRecover bool + err := job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover) + if err != nil { + return errors.Trace(err) + } + if enableGCAfterRecover { + err = enableGC(w) + if err != nil { + return errors.Trace(err) + } + } + return nil +} + func isDependencyJobDone(t *meta.Meta, job *model.Job) (bool, error) { if job.DependencyID == noneDependencyJob { return true, nil diff --git a/ddl/serial_test.go b/ddl/serial_test.go index d846cdafe7df1..0942b4fb76162 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -15,6 +15,7 @@ package ddl_test import ( "context" + "fmt" "time" . "github.com/pingcap/check" @@ -123,3 +124,67 @@ func (s *testSerialSuite) TestCancelAddIndexPanic(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[ddl:12]cancelled DDL job") } + +func (s *testSerialSuite) TestRestoreTableFail(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + rs, err := tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row := rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID := row.GetInt64(0) + + // enableGC first + err = admin.EnableGCAfterRecover(tk.Se) + c.Assert(err, IsNil) + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // set hook + hook := &ddl.TestDDLCallback{} + hook.OnJobRunBeforeExported = func(job *model.Job) { + if job.Type == model.ActionRestoreTable { + gofail.Enable("github.com/pingcap/tidb/store/tikv/mockCommitError", `return(true)`) + } + } + origHook := s.dom.DDL().GetHook() + defer s.dom.DDL().(ddl.DDLForTest).SetHook(origHook) + s.dom.DDL().(ddl.DDLForTest).SetHook(hook) + + // do restore table. + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + +} diff --git a/ddl/table.go b/ddl/table.go index a5cf291b72f61..fd59394d88a75 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -192,12 +192,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in if err != nil { return ver, errors.Trace(err) } - if enableGCAfterRecover { - err = enableGC(w) - if err != nil { - return ver, errors.Trace(err) - } - } + // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) return ver, nil diff --git a/store/tikv/txn.go b/store/tikv/txn.go index e19dec8bf50ba..0de97b1fcdb45 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -163,12 +163,22 @@ func (txn *tikvTxn) DelOption(opt kv.Option) { txn.us.DelOption(opt) } +// mockCommitErrorOnce use to make sure `mockCommitError` only mock error once. +var mockCommitErrorOnce = true + func (txn *tikvTxn) Commit(ctx context.Context) error { if !txn.valid { return kv.ErrInvalidTxn } defer txn.close() + // gofail: var mockCommitError bool + // if mockCommitError && mockCommitErrorOnce { + // mockCommitErrorOnce = false + // fmt.Println("go fail test cs") + // return errors.New("mock commit error") + // } + metrics.TiKVTxnCmdCounter.WithLabelValues("set").Add(float64(txn.setCnt)) metrics.TiKVTxnCmdCounter.WithLabelValues("commit").Inc() start := time.Now() From 3f34fb6e33b43bf2339bb6d44bbc162e45c2b885 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 20:25:03 +0800 Subject: [PATCH 20/48] update comment --- store/tikv/txn.go | 1 - 1 file changed, 1 deletion(-) diff --git a/store/tikv/txn.go b/store/tikv/txn.go index 0de97b1fcdb45..a266c173154b0 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -175,7 +175,6 @@ func (txn *tikvTxn) Commit(ctx context.Context) error { // gofail: var mockCommitError bool // if mockCommitError && mockCommitErrorOnce { // mockCommitErrorOnce = false - // fmt.Println("go fail test cs") // return errors.New("mock commit error") // } From 9af52a7724e6530f3ab0c7215d2f010630e7aae7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 20:46:06 +0800 Subject: [PATCH 21/48] refine gofail test --- ddl/serial_test.go | 2 ++ ddl/table.go | 9 +++++++++ kv/txn.go | 3 +++ store/tikv/txn.go | 7 ++----- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/ddl/serial_test.go b/ddl/serial_test.go index 0942b4fb76162..200717e22b875 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -171,6 +171,7 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { hook.OnJobRunBeforeExported = func(job *model.Job) { if job.Type == model.ActionRestoreTable { gofail.Enable("github.com/pingcap/tidb/store/tikv/mockCommitError", `return(true)`) + gofail.Enable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr", `return(true)`) } } origHook := s.dom.DDL().GetHook() @@ -180,6 +181,7 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { // do restore table. tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") + gofail.Disable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr") // check recover table meta and data record. tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) diff --git a/ddl/table.go b/ddl/table.go index fd59394d88a75..ad63d64006c87 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -148,6 +148,9 @@ func onDropTableOrView(t *meta.Meta, job *model.Job) (ver int64, _ error) { return ver, errors.Trace(err) } +// mockRestoreTableCommitErrOnce use to make sure `mockRestoreTableCommitErr` only mock error once. +var mockRestoreTableCommitErrOnce = true + func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { // check gc enable status again. gcEnable, err := checkGCEnable(w) @@ -193,6 +196,12 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in return ver, errors.Trace(err) } + // gofail: var mockRestoreTableCommitErr bool + // if mockRestoreTableCommitErr && mockRestoreTableCommitErrOnce { + // mockRestoreTableCommitErrOnce = false + // kv.MockCommitErrorEnable = true + // } + // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) return ver, nil diff --git a/kv/txn.go b/kv/txn.go index ff2360778eed6..2b44e5566e4ff 100644 --- a/kv/txn.go +++ b/kv/txn.go @@ -123,3 +123,6 @@ func BatchGetValues(txn Transaction, keys []Key) (map[string][]byte, error) { } return storageValues, nil } + +// MockCommitErrorEnable use to enable `mockCommitError` and only mock error once. +var MockCommitErrorEnable = true diff --git a/store/tikv/txn.go b/store/tikv/txn.go index a266c173154b0..6e65bb02f60e9 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -163,9 +163,6 @@ func (txn *tikvTxn) DelOption(opt kv.Option) { txn.us.DelOption(opt) } -// mockCommitErrorOnce use to make sure `mockCommitError` only mock error once. -var mockCommitErrorOnce = true - func (txn *tikvTxn) Commit(ctx context.Context) error { if !txn.valid { return kv.ErrInvalidTxn @@ -173,8 +170,8 @@ func (txn *tikvTxn) Commit(ctx context.Context) error { defer txn.close() // gofail: var mockCommitError bool - // if mockCommitError && mockCommitErrorOnce { - // mockCommitErrorOnce = false + // if mockCommitError && kv.MockCommitErrorEnable { + // kv.MockCommitErrorEnable = false // return errors.New("mock commit error") // } From af8ebcb9fdeb98ddbea56e10cc591cfbf8a3332b Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 21:09:03 +0800 Subject: [PATCH 22/48] fix data race in gofail test --- ddl/table.go | 2 +- kv/txn.go | 2 +- store/tikv/txn.go | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/ddl/table.go b/ddl/table.go index ad63d64006c87..f31c3100fede1 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -199,7 +199,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // gofail: var mockRestoreTableCommitErr bool // if mockRestoreTableCommitErr && mockRestoreTableCommitErrOnce { // mockRestoreTableCommitErrOnce = false - // kv.MockCommitErrorEnable = true + // atomic.StoreInt64(&kv.MockCommitErrorEnable,1) // } // Finish this job. diff --git a/kv/txn.go b/kv/txn.go index 2b44e5566e4ff..6d7017a0797f5 100644 --- a/kv/txn.go +++ b/kv/txn.go @@ -125,4 +125,4 @@ func BatchGetValues(txn Transaction, keys []Key) (map[string][]byte, error) { } // MockCommitErrorEnable use to enable `mockCommitError` and only mock error once. -var MockCommitErrorEnable = true +var MockCommitErrorEnable = int64(1) diff --git a/store/tikv/txn.go b/store/tikv/txn.go index 6e65bb02f60e9..6ad44339fe517 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -170,8 +170,8 @@ func (txn *tikvTxn) Commit(ctx context.Context) error { defer txn.close() // gofail: var mockCommitError bool - // if mockCommitError && kv.MockCommitErrorEnable { - // kv.MockCommitErrorEnable = false + // if mockCommitError && atomic.LoadInt64(&kv.MockCommitErrorEnable) == 1 { + // atomic.StoreInt64(&kv.MockCommitErrorEnable, 0) // return errors.New("mock commit error") // } From 795607c228250584b8081d8bac4809f6f852d061 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Thu, 10 Jan 2019 21:20:12 +0800 Subject: [PATCH 23/48] refine gofail test --- ddl/table.go | 2 +- kv/txn.go | 20 ++++++++++++++++++-- store/tikv/txn.go | 4 ++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/ddl/table.go b/ddl/table.go index f31c3100fede1..6914c1ff47a89 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -199,7 +199,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // gofail: var mockRestoreTableCommitErr bool // if mockRestoreTableCommitErr && mockRestoreTableCommitErrOnce { // mockRestoreTableCommitErrOnce = false - // atomic.StoreInt64(&kv.MockCommitErrorEnable,1) + // kv.MockCommitErrorEnable() // } // Finish this job. diff --git a/kv/txn.go b/kv/txn.go index 6d7017a0797f5..085fe640782be 100644 --- a/kv/txn.go +++ b/kv/txn.go @@ -17,6 +17,7 @@ import ( "context" "math" "math/rand" + "sync/atomic" "time" "github.com/pingcap/errors" @@ -124,5 +125,20 @@ func BatchGetValues(txn Transaction, keys []Key) (map[string][]byte, error) { return storageValues, nil } -// MockCommitErrorEnable use to enable `mockCommitError` and only mock error once. -var MockCommitErrorEnable = int64(1) +// mockCommitErrorEnable use to enable `mockCommitError` and only mock error once. +var mockCommitErrorEnable = int64(1) + +// MockCommitErrorEnable exports for gofail testing. +func MockCommitErrorEnable() { + atomic.StoreInt64(&mockCommitErrorEnable, 1) +} + +// MockCommitErrorDisable exports for gofail testing. +func MockCommitErrorDisable() { + atomic.StoreInt64(&mockCommitErrorEnable, 0) +} + +// IsMockCommitErrorEnable exports for gofail testing. +func IsMockCommitErrorEnable() bool { + return atomic.LoadInt64(&mockCommitErrorEnable) == 1 +} diff --git a/store/tikv/txn.go b/store/tikv/txn.go index 6ad44339fe517..33222f0778807 100644 --- a/store/tikv/txn.go +++ b/store/tikv/txn.go @@ -170,8 +170,8 @@ func (txn *tikvTxn) Commit(ctx context.Context) error { defer txn.close() // gofail: var mockCommitError bool - // if mockCommitError && atomic.LoadInt64(&kv.MockCommitErrorEnable) == 1 { - // atomic.StoreInt64(&kv.MockCommitErrorEnable, 0) + // if mockCommitError && kv.IsMockCommitErrorEnable() { + // kv.MockCommitErrorDisable() // return errors.New("mock commit error") // } From 180d0a49d06a5672763b6709a5d0541fda2fef5c Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 11:27:37 +0800 Subject: [PATCH 24/48] refine check gc logic --- ddl/db_test.go | 7 ++-- ddl/ddl.go | 2 +- ddl/ddl_api.go | 4 +-- ddl/ddl_worker.go | 3 +- ddl/serial_test.go | 3 +- ddl/table.go | 60 +++++++++++++++++++++++++-------- executor/admin.go | 28 +++++----------- executor/set.go | 3 +- kv/txn.go | 2 +- util/admin/admin.go | 33 ------------------ util/gcutil/gcutil.go | 78 +++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 148 insertions(+), 75 deletions(-) create mode 100644 util/gcutil/gcutil.go diff --git a/ddl/db_test.go b/ddl/db_test.go index eaf47b857234e..3a17c1334a806 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -47,6 +47,7 @@ import ( "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/admin" + "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/schemautil" "github.com/pingcap/tidb/util/testkit" @@ -2171,7 +2172,7 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "can not get 'tikv_gc_enable'") - err = admin.EnableGCAfterRecover(tk.Se) + err = gcutil.EnableGC(tk.Se) c.Assert(err, IsNil) // if gc safe point is not exists in mysql.tidb @@ -2200,7 +2201,7 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(err, NotNil) // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. - err = admin.DisableGCForRecover(tk.Se) + err = gcutil.DisableGC(tk.Se) c.Assert(err, IsNil) tk.MustExec("delete from t_recover where a > 1") @@ -2222,7 +2223,7 @@ func (s *testDBSuite) TestRestoreTable(c *C) { tk.MustExec("insert into t_recover values (7),(8),(9)") tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) - gcEnable, err := admin.CheckGCEnableStatus(tk.Se) + gcEnable, err := gcutil.CheckGCEnable(tk.Se) c.Assert(err, IsNil) c.Assert(gcEnable, Equals, false) diff --git a/ddl/ddl.go b/ddl/ddl.go index 0d501fdbd2dab..2f7d94617814b 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -215,7 +215,7 @@ type DDL interface { CreateView(ctx sessionctx.Context, stmt *ast.CreateViewStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) - RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64, enableGCAfterRecover bool) (err error) DropView(ctx sessionctx.Context, tableIdent ast.Ident) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index c6a716788b968..47036ee94cae3 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1023,13 +1023,13 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } -func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, enableGCAfterRecover bool) (err error) { +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64, enableGCAfterRecover bool) (err error) { job := &model.Job{ SchemaID: schemaID, TableID: tbInfo.ID, Type: model.ActionRestoreTable, BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tbInfo, autoID, dropJobID, enableGCAfterRecover}, + Args: []interface{}{tbInfo, autoID, dropJobID, snapshotTS, enableGCAfterRecover}, } err = d.doDDLJob(ctx, job) return errors.Trace(err) diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index 5ff746529ee87..1e0e91e34805b 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -298,8 +298,9 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { func finishRestoreTable(w *worker, t *meta.Meta, job *model.Job) error { tbInfo := &model.TableInfo{} var autoID, dropJobID int64 + var snapshotTS uint64 var enableGCAfterRecover bool - err := job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover) + err := job.DecodeArgs(tbInfo, &autoID, &dropJobID, &snapshotTS, &enableGCAfterRecover) if err != nil { return errors.Trace(err) } diff --git a/ddl/serial_test.go b/ddl/serial_test.go index 200717e22b875..d90e64048aae8 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -28,6 +28,7 @@ import ( "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/util/admin" + "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/testkit" "github.com/pingcap/tidb/util/testleak" @@ -162,7 +163,7 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { jobID := row.GetInt64(0) // enableGC first - err = admin.EnableGCAfterRecover(tk.Se) + err = gcutil.EnableGC(tk.Se) c.Assert(err, IsNil) tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) diff --git a/ddl/table.go b/ddl/table.go index 6914c1ff47a89..aa96d746e27c0 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -28,7 +28,7 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/tablecodec" - "github.com/pingcap/tidb/util/admin" + "github.com/pingcap/tidb/util/gcutil" log "github.com/sirupsen/logrus" ) @@ -148,31 +148,42 @@ func onDropTableOrView(t *meta.Meta, job *model.Job) (ver int64, _ error) { return ver, errors.Trace(err) } -// mockRestoreTableCommitErrOnce use to make sure `mockRestoreTableCommitErr` only mock error once. -var mockRestoreTableCommitErrOnce = true - -func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { - // check gc enable status again. +func checkGCForRestoreTable(w *worker, snapshotTS uint64) error { gcEnable, err := checkGCEnable(w) if err != nil { - job.State = model.JobStateCancelled - return ver, errors.Trace(err) + return errors.Trace(err) } if gcEnable { - job.State = model.JobStateCancelled - return ver, errors.Errorf("can not restore deleted table when gc is enable") + if err = disableGC(w); err != nil { + return errors.Errorf("disable gc failed, try again later. err: %v", err) + } + } + err = checkSafePoint(w, snapshotTS) + if err != nil { + return errors.Trace(err) } + return nil +} +func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { schemaID := job.SchemaID tbInfo := &model.TableInfo{} var autoID, dropJobID int64 + var snapshotTS uint64 var enableGCAfterRecover bool - if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &enableGCAfterRecover); err != nil { + if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &snapshotTS, &enableGCAfterRecover); err != nil { // Invalid arguments, cancel this job. job.State = model.JobStateCancelled return ver, errors.Trace(err) } + // check gc and safe point + err = checkGCForRestoreTable(w, snapshotTS) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + err = checkTableNotExists(t, job, schemaID, tbInfo.Name.L) if err != nil { return ver, errors.Trace(err) @@ -207,6 +218,9 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in return ver, nil } +// mockRestoreTableCommitErrOnce use to make sure `mockRestoreTableCommitErr` only mock error once. +var mockRestoreTableCommitErrOnce = true + func enableGC(w *worker) error { ctx, err := w.sessPool.get() if err != nil { @@ -214,7 +228,17 @@ func enableGC(w *worker) error { } defer w.sessPool.put(ctx) - return admin.EnableGCAfterRecover(ctx) + return gcutil.EnableGC(ctx) +} + +func disableGC(w *worker) error { + ctx, err := w.sessPool.get() + if err != nil { + return errors.Trace(err) + } + defer w.sessPool.put(ctx) + + return gcutil.DisableGC(ctx) } func checkGCEnable(w *worker) (enable bool, err error) { @@ -224,7 +248,17 @@ func checkGCEnable(w *worker) (enable bool, err error) { } defer w.sessPool.put(ctx) - return admin.CheckGCEnableStatus(ctx) + return gcutil.CheckGCEnable(ctx) +} + +func checkSafePoint(w *worker, snapshotTS uint64) error { + ctx, err := w.sessPool.get() + if err != nil { + return errors.Trace(err) + } + defer w.sessPool.put(ctx) + + return gcutil.ValidateSnapshot(ctx, snapshotTS) } type splitableStore interface { diff --git a/executor/admin.go b/executor/admin.go index fa85ed6cab151..a99f1c02368a6 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -35,6 +35,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/admin" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" @@ -185,22 +186,6 @@ func (e *RestoreTableExec) Open(ctx context.Context) error { // Next implements the Executor Open interface. func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { - gcEnable, err := admin.CheckGCEnableStatus(e.ctx) - if err != nil { - return err - } - if gcEnable { - err = admin.DisableGCForRecover(e.ctx) - if err != nil { - return err - } - defer func() { - // if err == nil, should be enable gc in ddl owner. - if err != nil { - log.Error(admin.EnableGCAfterRecover(e.ctx)) - } - }() - } txn, err := e.ctx.Txn(true) if err != nil { return errors.Trace(err) @@ -214,11 +199,11 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) } if job.Type != model.ActionDropTable { - return errors.Errorf("Job %v doesn't drop any table", job.ID) + return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } // check gc safe point - err = validateSnapshot(e.ctx, job.StartTS) + err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { return errors.Trace(err) } @@ -246,8 +231,13 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { if err != nil { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } + // check gc enable use to decide whether enable gc after restore table. + gcEnable, err := gcutil.CheckGCEnable(e.ctx) + if err != nil { + return err + } // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, gcEnable) + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS, gcEnable) return errors.Trace(err) } diff --git a/executor/set.go b/executor/set.go index 9b67a749b735a..67a550af8e93c 100644 --- a/executor/set.go +++ b/executor/set.go @@ -29,6 +29,7 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/sqlexec" log "github.com/sirupsen/logrus" ) @@ -158,7 +159,7 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e } newSnapshotIsSet := sessionVars.SnapshotTS > 0 && sessionVars.SnapshotTS != oldSnapshotTS if newSnapshotIsSet { - err = validateSnapshot(e.ctx, sessionVars.SnapshotTS) + err = gcutil.ValidateSnapshot(e.ctx, sessionVars.SnapshotTS) if err != nil { sessionVars.SnapshotTS = oldSnapshotTS return errors.Trace(err) diff --git a/kv/txn.go b/kv/txn.go index 085fe640782be..ff35490746e77 100644 --- a/kv/txn.go +++ b/kv/txn.go @@ -126,7 +126,7 @@ func BatchGetValues(txn Transaction, keys []Key) (map[string][]byte, error) { } // mockCommitErrorEnable use to enable `mockCommitError` and only mock error once. -var mockCommitErrorEnable = int64(1) +var mockCommitErrorEnable = int64(0) // MockCommitErrorEnable exports for gofail testing. func MockCommitErrorEnable() { diff --git a/util/admin/admin.go b/util/admin/admin.go index a10dabad1815d..52e3eae298280 100644 --- a/util/admin/admin.go +++ b/util/admin/admin.go @@ -721,39 +721,6 @@ func iterRecords(sessCtx sessionctx.Context, retriever kv.Retriever, t table.Tab return nil } -// CheckGCEnableStatus is use to check whether gc is enable. -func CheckGCEnableStatus(ctx sessionctx.Context) (enable bool, err error) { - sql := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, "tikv_gc_enable") - rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) - if err != nil { - return false, errors.Trace(err) - } - if len(rows) != 1 { - return false, errors.New("can not get 'tikv_gc_enable'") - } - return rows[0].GetString(0) == "true", nil -} - -// DisableGCForRecover will disable gc enable variable. -func DisableGCForRecover(ctx sessionctx.Context) error { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') - ON DUPLICATE KEY - UPDATE variable_value = '%[2]s', comment = '%[3]s'`, - "tikv_gc_enable", "false", "Current GC enable status") - _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) - return errors.Trace(err) -} - -// EnableGCAfterRecover will enable gc enable variable. -func EnableGCAfterRecover(ctx sessionctx.Context) error { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') - ON DUPLICATE KEY - UPDATE variable_value = '%[2]s', comment = '%[3]s'`, - "tikv_gc_enable", "true", "Current GC enable status") - _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) - return errors.Trace(err) -} - // admin error codes. const ( codeDataNotEqual terror.ErrCode = 1 diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go new file mode 100644 index 0000000000000..4a29bf7312a63 --- /dev/null +++ b/util/gcutil/gcutil.go @@ -0,0 +1,78 @@ +// Copyright 2019 PingCAP, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// See the License for the specific language governing permissions and +// limitations under the License. + +package gcutil + +import ( + "fmt" + "github.com/pingcap/errors" + "github.com/pingcap/tidb/sessionctx" + "github.com/pingcap/tidb/sessionctx/variable" + "github.com/pingcap/tidb/util" + "github.com/pingcap/tidb/util/sqlexec" +) + +// CheckGCEnable is use to check whether gc is enable. +func CheckGCEnable(ctx sessionctx.Context) (enable bool, err error) { + sql := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, "tikv_gc_enable") + rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + if err != nil { + return false, errors.Trace(err) + } + if len(rows) != 1 { + return false, errors.New("can not get 'tikv_gc_enable'") + } + return rows[0].GetString(0) == "true", nil +} + +// DisableGC will disable gc enable variable. +func DisableGC(ctx sessionctx.Context) error { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') + ON DUPLICATE KEY + UPDATE variable_value = '%[2]s', comment = '%[3]s'`, + "tikv_gc_enable", "false", "Current GC enable status") + _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + return errors.Trace(err) +} + +// EnableGC will enable gc enable variable. +func EnableGC(ctx sessionctx.Context) error { + sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') + ON DUPLICATE KEY + UPDATE variable_value = '%[2]s', comment = '%[3]s'`, + "tikv_gc_enable", "true", "Current GC enable status") + _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + return errors.Trace(err) +} + +// ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. +func ValidateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { + sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" + rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + if err != nil { + return errors.Trace(err) + } + if len(rows) != 1 { + return errors.New("can not get 'tikv_gc_safe_point'") + } + safePointString := rows[0].GetString(0) + safePointTime, err := util.CompatibleParseGCTime(safePointString) + if err != nil { + return errors.Trace(err) + } + safePointTS := variable.GoTimeToTS(safePointTime) + if safePointTS > snapshotTS { + return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) + } + return nil +} From 8a25126b48d98738bd1292c1c02ffc1fcdd481fb Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 11:59:41 +0800 Subject: [PATCH 25/48] fix test --- executor/admin.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/executor/admin.go b/executor/admin.go index a99f1c02368a6..f5e8e523b38c7 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -202,6 +202,11 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } + // check gc enable use to decide whether enable gc after restore table. + gcEnable, err := gcutil.CheckGCEnable(e.ctx) + if err != nil { + return err + } // check gc safe point err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { @@ -231,11 +236,6 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { if err != nil { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } - // check gc enable use to decide whether enable gc after restore table. - gcEnable, err := gcutil.CheckGCEnable(e.ctx) - if err != nil { - return err - } // Call DDL RestoreTable err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS, gcEnable) return errors.Trace(err) From 0723678da8f339918bd34fa9830225725ca8daea Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 12:09:09 +0800 Subject: [PATCH 26/48] remove blank line --- ddl/db_test.go | 3 --- ddl/serial_test.go | 3 --- 2 files changed, 6 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 3a17c1334a806..3add9cdfcdc47 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2147,10 +2147,8 @@ func (s *testDBSuite) TestRestoreTable(c *C) { // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. ddl.EmulatorGCDisable() gcTimeFormat := "20060102-15:04:05 -0700 MST" - timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) - safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') ON DUPLICATE KEY UPDATE variable_value = '%[1]s'` @@ -2226,7 +2224,6 @@ func (s *testDBSuite) TestRestoreTable(c *C) { gcEnable, err := gcutil.CheckGCEnable(tk.Se) c.Assert(err, IsNil) c.Assert(gcEnable, Equals, false) - } func (s *testDBSuite) TestTransactionOnAddDropColumn(c *C) { diff --git a/ddl/serial_test.go b/ddl/serial_test.go index d90e64048aae8..9d075bd97bb52 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -143,9 +143,7 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. ddl.EmulatorGCDisable() gcTimeFormat := "20060102-15:04:05 -0700 MST" - timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) - safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') ON DUPLICATE KEY UPDATE variable_value = '%[1]s'` @@ -189,5 +187,4 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { // check recover table autoID. tk.MustExec("insert into t_recover values (4),(5),(6)") tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) - } From 89bda46365f05c6212f7c0467dae4002680f3451 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 13:42:45 +0800 Subject: [PATCH 27/48] move check gc enable to ddl owner --- ddl/db_test.go | 14 +++--- ddl/ddl.go | 2 +- ddl/ddl_api.go | 5 +- ddl/ddl_worker.go | 7 ++- ddl/serial_test.go | 5 ++ ddl/table.go | 115 ++++++++++++++++++++++++++++----------------- executor/admin.go | 8 +--- 7 files changed, 92 insertions(+), 64 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 3add9cdfcdc47..cad5a59e80fe1 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2165,19 +2165,21 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(row.GetString(3), Equals, "drop table") jobID := row.GetInt64(0) + // if gc safe point is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if gc enable is not exists in mysql.tidb _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "can not get 'tikv_gc_enable'") + c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") err = gcutil.EnableGC(tk.Se) c.Assert(err, IsNil) - // if gc safe point is not exists in mysql.tidb - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // recover job is before gc safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) diff --git a/ddl/ddl.go b/ddl/ddl.go index 2f7d94617814b..fe489aa70307a 100644 --- a/ddl/ddl.go +++ b/ddl/ddl.go @@ -215,7 +215,7 @@ type DDL interface { CreateView(ctx sessionctx.Context, stmt *ast.CreateViewStmt) error CreateTableWithLike(ctx sessionctx.Context, ident, referIdent ast.Ident, ifNotExists bool) error DropTable(ctx sessionctx.Context, tableIdent ast.Ident) (err error) - RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64, enableGCAfterRecover bool) (err error) + RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64) (err error) DropView(ctx sessionctx.Context, tableIdent ast.Ident) (err error) CreateIndex(ctx sessionctx.Context, tableIdent ast.Ident, unique bool, indexName model.CIStr, columnNames []*ast.IndexColName, indexOption *ast.IndexOption) error diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index 47036ee94cae3..bea50309fdb42 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1023,13 +1023,14 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e return errors.Trace(err) } -func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64, enableGCAfterRecover bool) (err error) { +func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64) (err error) { + tbInfo.State = model.StateNone job := &model.Job{ SchemaID: schemaID, TableID: tbInfo.ID, Type: model.ActionRestoreTable, BinlogInfo: &model.HistoryInfo{}, - Args: []interface{}{tbInfo, autoID, dropJobID, snapshotTS, enableGCAfterRecover}, + Args: []interface{}{tbInfo, autoID, dropJobID, snapshotTS, restoreTableCheckFlagNone}, } err = d.doDDLJob(ctx, job) return errors.Trace(err) diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index 1e0e91e34805b..6b3ff5bc4426b 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -297,14 +297,13 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { func finishRestoreTable(w *worker, t *meta.Meta, job *model.Job) error { tbInfo := &model.TableInfo{} - var autoID, dropJobID int64 + var autoID, dropJobID, restoreTableCheckFlag int64 var snapshotTS uint64 - var enableGCAfterRecover bool - err := job.DecodeArgs(tbInfo, &autoID, &dropJobID, &snapshotTS, &enableGCAfterRecover) + err := job.DecodeArgs(tbInfo, &autoID, &dropJobID, &snapshotTS, &restoreTableCheckFlag) if err != nil { return errors.Trace(err) } - if enableGCAfterRecover { + if restoreTableCheckFlag == restoreTableCheckFlagEnableGC { err = enableGC(w) if err != nil { return errors.Trace(err) diff --git a/ddl/serial_test.go b/ddl/serial_test.go index 9d075bd97bb52..a5fcfc3bfed5e 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -182,6 +182,11 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") gofail.Disable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr") + // make sure enable gc after restore table. + enable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(enable, Equals, true) + // check recover table meta and data record. tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) // check recover table autoID. diff --git a/ddl/table.go b/ddl/table.go index aa96d746e27c0..19ed7a32ad62c 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -148,73 +148,100 @@ func onDropTableOrView(t *meta.Meta, job *model.Job) (ver int64, _ error) { return ver, errors.Trace(err) } -func checkGCForRestoreTable(w *worker, snapshotTS uint64) error { - gcEnable, err := checkGCEnable(w) - if err != nil { - return errors.Trace(err) - } - if gcEnable { - if err = disableGC(w); err != nil { - return errors.Errorf("disable gc failed, try again later. err: %v", err) - } - } - err = checkSafePoint(w, snapshotTS) - if err != nil { - return errors.Trace(err) - } - return nil -} +const ( + restoreTableCheckFlagNone int64 = iota + restoreTableCheckFlagEnableGC + restoreTableCheckFlagDisableGC +) func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) { schemaID := job.SchemaID - tbInfo := &model.TableInfo{} - var autoID, dropJobID int64 + tblInfo := &model.TableInfo{} + var autoID, dropJobID, restoreTableCheckFlag int64 var snapshotTS uint64 - var enableGCAfterRecover bool - if err = job.DecodeArgs(tbInfo, &autoID, &dropJobID, &snapshotTS, &enableGCAfterRecover); err != nil { + if err = job.DecodeArgs(tblInfo, &autoID, &dropJobID, &snapshotTS, &restoreTableCheckFlag); err != nil { // Invalid arguments, cancel this job. job.State = model.JobStateCancelled return ver, errors.Trace(err) } // check gc and safe point - err = checkGCForRestoreTable(w, snapshotTS) + gcEnable, err := checkGCEnable(w) if err != nil { job.State = model.JobStateCancelled return ver, errors.Trace(err) } - err = checkTableNotExists(t, job, schemaID, tbInfo.Name.L) + err = checkTableNotExists(t, job, schemaID, tblInfo.Name.L) if err != nil { return ver, errors.Trace(err) } - ver, err = updateSchemaVersion(t, job) - if err != nil { - return ver, errors.Trace(err) - } + // Restore table divide into 2 steps: + // 1. Check gc enable status, to decided whether enable gc after restore table. + // 2. Do restore table job. + // 1. Check whether gc enabled, if enabled, disable gc first. + // 2. Check gc safe point. If drop table time if after safe point time, then can do restore. + // otherwise, can't restore table, because the records of the table may already delete by gc. + // 3. Remove gc task of the table from gc_delete_range table. + // 4. Create table and rebase table auto ID. + // 5. Finish. + switch tblInfo.State { + case model.StateNone: + // none -> write only + // check gc enable and update flag. + if gcEnable { + job.Args[len(job.Args)-1] = restoreTableCheckFlagEnableGC + } else { + job.Args[len(job.Args)-1] = restoreTableCheckFlagDisableGC + } - // Remove dropped table DDL job from gc_delete_range table. - err = w.delRangeManager.removeFromGCDeleteRange(dropJobID, tbInfo.ID) - if err != nil { - return ver, errors.Trace(err) - } + job.SchemaState = model.StateWriteOnly + tblInfo.State = model.StateWriteOnly + ver, err = updateVersionAndTableInfo(t, job, tblInfo, false) + case model.StateWriteOnly: + // write only -> public + // do restore table. + if gcEnable { + err = disableGC(w) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Errorf("disable gc failed, try again later. err: %v", err) + } + } + // check gc safe point + err = checkSafePoint(w, snapshotTS) + if err != nil { + job.State = model.JobStateCancelled + return ver, errors.Trace(err) + } + // Remove dropped table DDL job from gc_delete_range table. + err = w.delRangeManager.removeFromGCDeleteRange(dropJobID, tblInfo.ID) + if err != nil { + return ver, errors.Trace(err) + } - tbInfo.State = model.StatePublic - tbInfo.UpdateTS = t.StartTS - err = t.CreateTableAndSetAutoID(schemaID, tbInfo, autoID) - if err != nil { - return ver, errors.Trace(err) - } + tblInfo.State = model.StatePublic + tblInfo.UpdateTS = t.StartTS + err = t.CreateTableAndSetAutoID(schemaID, tblInfo, autoID) + if err != nil { + return ver, errors.Trace(err) + } - // gofail: var mockRestoreTableCommitErr bool - // if mockRestoreTableCommitErr && mockRestoreTableCommitErrOnce { - // mockRestoreTableCommitErrOnce = false - // kv.MockCommitErrorEnable() - // } + // gofail: var mockRestoreTableCommitErr bool + // if mockRestoreTableCommitErr && mockRestoreTableCommitErrOnce { + // mockRestoreTableCommitErrOnce = false + // kv.MockCommitErrorEnable() + // } - // Finish this job. - job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tbInfo) + ver, err = updateVersionAndTableInfo(t, job, tblInfo, true) + if err != nil { + return ver, errors.Trace(err) + } + + // Finish this job. + job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo) + } return ver, nil } diff --git a/executor/admin.go b/executor/admin.go index f5e8e523b38c7..a81a2273c7c8d 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -202,11 +202,6 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } - // check gc enable use to decide whether enable gc after restore table. - gcEnable, err := gcutil.CheckGCEnable(e.ctx) - if err != nil { - return err - } // check gc safe point err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { @@ -237,9 +232,8 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) } // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS, gcEnable) + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS) return errors.Trace(err) - } // RecoverIndexExec represents a recover index executor. From 21d8e1a7724abee2f288ee5c60a31ceed2e4356e Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 15:04:19 +0800 Subject: [PATCH 28/48] add comment --- ddl/table.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ddl/table.go b/ddl/table.go index 19ed7a32ad62c..775d9b6851ffb 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -179,6 +179,13 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // Restore table divide into 2 steps: // 1. Check gc enable status, to decided whether enable gc after restore table. + // 1. Why not disable gc before put the job to ddl job queue? + // Think about concurrency problem. If a restore job-1 is doing and already disabled GC, + // then, another restore table job-2 check gc enable will get disable before into the job queue. + // then, after restore table job-2 finished, the gc will be disabled. + // 2. Why split into 2 steps? 1 step also can finish this job: check gc -> disable gc -> restore table -> finish job. + // What if the transaction commit failed? then, the job will retry, but the gc already disabled when first running. + // So, after this job retry succeed, the gc will be disabled. // 2. Do restore table job. // 1. Check whether gc enabled, if enabled, disable gc first. // 2. Check gc safe point. If drop table time if after safe point time, then can do restore. From 8cfd77951ec679e261116289270e562467ef356f Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Fri, 11 Jan 2019 19:32:31 +0800 Subject: [PATCH 29/48] address comment --- ddl/db_test.go | 11 ++++++- executor/admin.go | 73 ----------------------------------------------- executor/ddl.go | 71 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 74 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index cad5a59e80fe1..47508fd4c4950 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2186,8 +2186,17 @@ func (s *testDBSuite) TestRestoreTable(c *C) { c.Assert(err, NotNil) c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) - // recover job after gc safe point + // set gc safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if there is a new table with the same name, should return failed. + tk.MustExec("create table t_recover (a int);") + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) + + // drop the new table with the same name, then restore table. + tk.MustExec("drop table t_recover") + + // do restore table. tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) // check recover table meta and data record. diff --git a/executor/admin.go b/executor/admin.go index a81a2273c7c8d..6759dd43945df 100644 --- a/executor/admin.go +++ b/executor/admin.go @@ -15,7 +15,6 @@ package executor import ( "context" - "fmt" "math" "github.com/pingcap/errors" @@ -23,19 +22,15 @@ import ( "github.com/pingcap/parser/model" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/distsql" - "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" - "github.com/pingcap/tidb/meta" plannercore "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/statistics" "github.com/pingcap/tidb/table" "github.com/pingcap/tidb/table/tables" "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" - "github.com/pingcap/tidb/util/admin" "github.com/pingcap/tidb/util/chunk" - "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/ranger" "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" @@ -168,74 +163,6 @@ func (e *CheckIndexRangeExec) Close() error { return nil } -// RestoreTableExec represents a recover table executor. -// It is built from "admin restore table by job" statement, -// is used to recover the table that deleted by mistake. -type RestoreTableExec struct { - baseExecutor - jobID int64 -} - -// Open implements the Executor Open interface. -func (e *RestoreTableExec) Open(ctx context.Context) error { - if err := e.baseExecutor.Open(ctx); err != nil { - return errors.Trace(err) - } - return nil -} - -// Next implements the Executor Open interface. -func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { - txn, err := e.ctx.Txn(true) - if err != nil { - return errors.Trace(err) - } - t := meta.NewMeta(txn) - job, err := t.GetHistoryDDLJob(e.jobID) - if err != nil { - return errors.Trace(err) - } - if job == nil { - return admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) - } - if job.Type != model.ActionDropTable { - return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) - } - - // check gc safe point - err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) - if err != nil { - return errors.Trace(err) - } - - dom := domain.GetDomain(e.ctx) - // Get the snapshot infoSchema before drop table. - snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) - if err != nil { - return errors.Trace(err) - } - // Get table meta from snapshot infoSchema. - table, ok := snapInfo.TableByID(job.TableID) - if !ok { - return infoschema.ErrTableNotExists.GenWithStackByArgs( - fmt.Sprintf("(Schema ID %d)", job.SchemaID), - fmt.Sprintf("(Table ID %d)", job.TableID), - ) - } - // Get table original autoID before table drop. - m, err := dom.GetSnapshotMeta(job.StartTS) - if err != nil { - return errors.Trace(err) - } - autoID, err := m.GetAutoTableID(job.SchemaID, job.TableID) - if err != nil { - return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) - } - // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS) - return errors.Trace(err) -} - // RecoverIndexExec represents a recover index executor. // It is built from "admin recover index" statement, is used to backfill // corrupted index. diff --git a/executor/ddl.go b/executor/ddl.go index 05c4f18eba002..dfaa67b77954f 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -25,9 +25,12 @@ import ( "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" + "github.com/pingcap/tidb/meta" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" + "github.com/pingcap/tidb/util/admin" "github.com/pingcap/tidb/util/chunk" + "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/sqlexec" log "github.com/sirupsen/logrus" ) @@ -287,3 +290,71 @@ func (e *DDLExec) executeAlterTable(s *ast.AlterTableStmt) error { err := domain.GetDomain(e.ctx).DDL().AlterTable(e.ctx, ti, s.Specs) return errors.Trace(err) } + +// RestoreTableExec represents a recover table executor. +// It is built from "admin restore table by job" statement, +// is used to recover the table that deleted by mistake. +type RestoreTableExec struct { + baseExecutor + jobID int64 +} + +// Open implements the Executor Open interface. +func (e *RestoreTableExec) Open(ctx context.Context) error { + if err := e.baseExecutor.Open(ctx); err != nil { + return errors.Trace(err) + } + return nil +} + +// Next implements the Executor Open interface. +func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { + txn, err := e.ctx.Txn(true) + if err != nil { + return errors.Trace(err) + } + t := meta.NewMeta(txn) + job, err := t.GetHistoryDDLJob(e.jobID) + if err != nil { + return errors.Trace(err) + } + if job == nil { + return admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) + } + if job.Type != model.ActionDropTable { + return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) + } + + // check gc safe point + err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) + if err != nil { + return errors.Trace(err) + } + + dom := domain.GetDomain(e.ctx) + // Get the snapshot infoSchema before drop table. + snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) + if err != nil { + return errors.Trace(err) + } + // Get table meta from snapshot infoSchema. + table, ok := snapInfo.TableByID(job.TableID) + if !ok { + return infoschema.ErrTableNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", job.SchemaID), + fmt.Sprintf("(Table ID %d)", job.TableID), + ) + } + // Get table original autoID before table drop. + m, err := dom.GetSnapshotMeta(job.StartTS) + if err != nil { + return errors.Trace(err) + } + autoID, err := m.GetAutoTableID(job.SchemaID, job.TableID) + if err != nil { + return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) + } + // Call DDL RestoreTable + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS) + return errors.Trace(err) +} From 68453bc6ec43efc47162c8bc0af2fe014e1d00da Mon Sep 17 00:00:00 2001 From: crazycs Date: Fri, 11 Jan 2019 22:28:49 +0800 Subject: [PATCH 30/48] update comment --- executor/ddl.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/executor/ddl.go b/executor/ddl.go index dfaa67b77954f..8a3c3f41a86dd 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -325,7 +325,7 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } - // check gc safe point + // Check gc safe point for getting snapshot infoSchema. err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { return errors.Trace(err) From fbd1e17f896c4c05e1db9863f6d4ed0dd977c130 Mon Sep 17 00:00:00 2001 From: crazycs Date: Sat, 12 Jan 2019 01:10:31 +0800 Subject: [PATCH 31/48] clean code --- executor/set.go | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/executor/set.go b/executor/set.go index 67a550af8e93c..1377a9c533121 100644 --- a/executor/set.go +++ b/executor/set.go @@ -24,13 +24,10 @@ import ( "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/expression" - "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" - "github.com/pingcap/tidb/util" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/gcutil" - "github.com/pingcap/tidb/util/sqlexec" log "github.com/sirupsen/logrus" ) @@ -184,28 +181,6 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e return nil } -// validateSnapshot checks that the newly set snapshot time is after GC safe point time. -func validateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { - sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" - rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) - if err != nil { - return errors.Trace(err) - } - if len(rows) != 1 { - return errors.New("can not get 'tikv_gc_safe_point'") - } - safePointString := rows[0].GetString(0) - safePointTime, err := util.CompatibleParseGCTime(safePointString) - if err != nil { - return errors.Trace(err) - } - safePointTS := variable.GoTimeToTS(safePointTime) - if safePointTS > snapshotTS { - return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) - } - return nil -} - func (e *SetExecutor) setCharset(cs, co string) error { var err error if len(co) == 0 { From 29ac1a1032780def97c453a44b4413f05bebcede Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 12 Jan 2019 21:58:12 +0800 Subject: [PATCH 32/48] add syntax: restore table by job id --- ddl/db_test.go | 310 +++++++++++++++++++++++------------ ddl/serial_test.go | 61 +++++++ executor/builder.go | 2 + executor/ddl.go | 93 +++++++++-- go.mod | 2 + go.sum | 2 + planner/core/common_plans.go | 4 +- planner/core/planbuilder.go | 8 +- util/gcutil/gcutil.go | 38 ++++- 9 files changed, 389 insertions(+), 131 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 47508fd4c4950..bc1abfe0cd2e7 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2130,113 +2130,6 @@ LOOP: s.mustExec(c, "drop table t1") } -func (s *testDBSuite) TestRestoreTable(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("create database if not exists test_restore") - tk.MustExec("use test_restore") - tk.MustExec("create table t_recover (a int);") - defer func(originGC bool) { - if originGC { - ddl.EmulatorGCEnable() - } else { - ddl.EmulatorGCDisable() - } - }(ddl.IsEmulatorGCEnable()) - - // disable emulator GC. - // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. - ddl.EmulatorGCDisable() - gcTimeFormat := "20060102-15:04:05 -0700 MST" - timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) - timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) - safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') - ON DUPLICATE KEY - UPDATE variable_value = '%[1]s'` - - tk.MustExec("insert into t_recover values (1),(2),(3)") - tk.MustExec("drop table t_recover") - - rs, err := tk.Exec("admin show ddl jobs") - c.Assert(err, IsNil) - rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) - c.Assert(err, IsNil) - row := rows[0] - c.Assert(row.GetString(1), Equals, "test_restore") - c.Assert(row.GetString(3), Equals, "drop table") - jobID := row.GetInt64(0) - - // if gc safe point is not exists in mysql.tidb - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - - // if gc enable is not exists in mysql.tidb - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") - - err = gcutil.EnableGC(tk.Se) - c.Assert(err, IsNil) - - // recover job is before gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) - - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - // if there is a new table with the same name, should return failed. - tk.MustExec("create table t_recover (a int);") - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) - - // drop the new table with the same name, then restore table. - tk.MustExec("drop table t_recover") - - // do restore table. - tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (4),(5),(6)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) - - // restore table by none exits job. - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) - c.Assert(err, NotNil) - - // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. - err = gcutil.DisableGC(tk.Se) - c.Assert(err, IsNil) - - tk.MustExec("delete from t_recover where a > 1") - tk.MustExec("drop table t_recover") - rs, err = tk.Exec("admin show ddl jobs") - c.Assert(err, IsNil) - rows, err = session.GetRows4Test(context.Background(), tk.Se, rs) - c.Assert(err, IsNil) - row = rows[0] - c.Assert(row.GetString(1), Equals, "test_restore") - c.Assert(row.GetString(3), Equals, "drop table") - jobID = row.GetInt64(0) - - tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (7),(8),(9)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) - - gcEnable, err := gcutil.CheckGCEnable(tk.Se) - c.Assert(err, IsNil) - c.Assert(gcEnable, Equals, false) -} - func (s *testDBSuite) TestTransactionOnAddDropColumn(c *C) { s.tk = testkit.NewTestKit(c, s.store) s.mustExec(c, "use test_db") @@ -2407,3 +2300,206 @@ func (s *testDBSuite) TestAddColumn2(c *C) { re.Check(testkit.Rows("1 2")) s.tk.MustQuery("select a,b,_tidb_rowid from t2").Check(testkit.Rows("1 3 2")) } + +func (s *testDBSuite) TestRestoreTable(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + // clear gc variables first. + tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + rs, err := tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row := rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID := row.GetInt64(0) + + // if gc safe point is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // if gc enable is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") + + err = gcutil.EnableGC(tk.Se) + c.Assert(err, IsNil) + + // recover job is before gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) + + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if there is a new table with the same name, should return failed. + tk.MustExec("create table t_recover (a int);") + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) + + // drop the new table with the same name, then restore table. + tk.MustExec("drop table t_recover") + + // do restore table. + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + + // restore table by none exits job. + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) + c.Assert(err, NotNil) + + // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + err = gcutil.DisableGC(tk.Se) + c.Assert(err, IsNil) + + tk.MustExec("delete from t_recover where a > 1") + tk.MustExec("drop table t_recover") + rs, err = tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err = session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row = rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID = row.GetInt64(0) + + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (7),(8),(9)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) + + gcEnable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(gcEnable, Equals, false) +} + +func (s *testDBSuite) TestRestoreTableByTableName(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover, t_recover2") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + // clear gc variables first. + tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + // if gc safe point is not exists in mysql.tidb + _, err := tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // if gc enable is not exists in mysql.tidb + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") + + err = gcutil.EnableGC(tk.Se) + c.Assert(err, IsNil) + + // recover job is before gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) + + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if there is a new table with the same name, should return failed. + tk.MustExec("create table t_recover (a int);") + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) + + // drop the new table with the same name, then restore table. + tk.MustExec("rename table t_recover to t_recover2") + + // do restore table. + tk.MustExec("admin restore table t_recover") + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + + // restore table by none exits job. + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) + c.Assert(err, NotNil) + + // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + err = gcutil.DisableGC(tk.Se) + c.Assert(err, IsNil) + + tk.MustExec("delete from t_recover where a > 1") + tk.MustExec("drop table t_recover") + + tk.MustExec("admin restore table t_recover") + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (7),(8),(9)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) + + gcEnable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(gcEnable, Equals, false) +} diff --git a/ddl/serial_test.go b/ddl/serial_test.go index a5fcfc3bfed5e..f906df2b19473 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -130,6 +130,7 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover") tk.MustExec("create table t_recover (a int);") defer func(originGC bool) { if originGC { @@ -193,3 +194,63 @@ func (s *testSerialSuite) TestRestoreTableFail(c *C) { tk.MustExec("insert into t_recover values (4),(5),(6)") tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) } + +func (s *testSerialSuite) TestRestoreTableFailByTableName(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + // enableGC first + err := gcutil.EnableGC(tk.Se) + c.Assert(err, IsNil) + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // set hook + hook := &ddl.TestDDLCallback{} + hook.OnJobRunBeforeExported = func(job *model.Job) { + if job.Type == model.ActionRestoreTable { + gofail.Enable("github.com/pingcap/tidb/store/tikv/mockCommitError", `return(true)`) + gofail.Enable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr", `return(true)`) + } + } + origHook := s.dom.DDL().GetHook() + defer s.dom.DDL().(ddl.DDLForTest).SetHook(origHook) + s.dom.DDL().(ddl.DDLForTest).SetHook(hook) + + // do restore table. + tk.MustExec("admin restore table t_recover") + gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") + gofail.Disable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr") + + // make sure enable gc after restore table. + enable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(enable, Equals, true) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) +} diff --git a/executor/builder.go b/executor/builder.go index 0a1e895c6ff86..b07efcbbbad1c 100644 --- a/executor/builder.go +++ b/executor/builder.go @@ -341,6 +341,8 @@ func (b *executorBuilder) buildRestoreTable(v *plannercore.RestoreTable) Executo e := &RestoreTableExec{ baseExecutor: newBaseExecutor(b.ctx, v.Schema(), v.ExplainID()), jobID: v.JobID, + Table: v.Table, + JobNum: v.JobNum, } return e } diff --git a/executor/ddl.go b/executor/ddl.go index 8a3c3f41a86dd..3eeab255cd6a5 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -296,7 +296,9 @@ func (e *DDLExec) executeAlterTable(s *ast.AlterTableStmt) error { // is used to recover the table that deleted by mistake. type RestoreTableExec struct { baseExecutor - jobID int64 + jobID int64 + Table *ast.TableName + JobNum int64 } // Open implements the Executor Open interface. @@ -314,47 +316,106 @@ func (e *RestoreTableExec) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Trace(err) } t := meta.NewMeta(txn) - job, err := t.GetHistoryDDLJob(e.jobID) + dom := domain.GetDomain(e.ctx) + var job *model.Job + var tblInfo *model.TableInfo + if e.jobID != 0 { + job, tblInfo, err = getRestoreTableByJobID(e, t, dom) + } else { + job, tblInfo, err = getRestoreTableByTableName(e, t, dom) + } if err != nil { return errors.Trace(err) } + // Get table original autoID before table drop. + m, err := dom.GetSnapshotMeta(job.StartTS) + if err != nil { + return errors.Trace(err) + } + autoID, err := m.GetAutoTableID(job.SchemaID, job.TableID) + if err != nil { + return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) + } + // Call DDL RestoreTable + err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, tblInfo, job.SchemaID, autoID, job.ID, job.StartTS) + return errors.Trace(err) +} + +func getRestoreTableByJobID(e *RestoreTableExec, t *meta.Meta, dom *domain.Domain) (*model.Job, *model.TableInfo, error) { + job, err := t.GetHistoryDDLJob(e.jobID) + if err != nil { + return nil, nil, errors.Trace(err) + } if job == nil { - return admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) + return nil, nil, admin.ErrDDLJobNotFound.GenWithStackByArgs(e.jobID) } if job.Type != model.ActionDropTable { - return errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) + return nil, nil, errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } // Check gc safe point for getting snapshot infoSchema. err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { - return errors.Trace(err) + return nil, nil, errors.Trace(err) } - dom := domain.GetDomain(e.ctx) // Get the snapshot infoSchema before drop table. snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) if err != nil { - return errors.Trace(err) + return nil, nil, errors.Trace(err) } // Get table meta from snapshot infoSchema. table, ok := snapInfo.TableByID(job.TableID) if !ok { - return infoschema.ErrTableNotExists.GenWithStackByArgs( + return nil, nil, infoschema.ErrTableNotExists.GenWithStackByArgs( fmt.Sprintf("(Schema ID %d)", job.SchemaID), fmt.Sprintf("(Table ID %d)", job.TableID), ) } - // Get table original autoID before table drop. - m, err := dom.GetSnapshotMeta(job.StartTS) + return job, table.Meta(), nil +} + +func getRestoreTableByTableName(e *RestoreTableExec, t *meta.Meta, dom *domain.Domain) (*model.Job, *model.TableInfo, error) { + jobs, err := t.GetAllHistoryDDLJobs() if err != nil { - return errors.Trace(err) + return nil, nil, errors.Trace(err) } - autoID, err := m.GetAutoTableID(job.SchemaID, job.TableID) + var job *model.Job + var tblInfo *model.TableInfo + gcSafePoint, err := gcutil.GetGCSafePoint(e.ctx) if err != nil { - return errors.Errorf("recover table_id: %d, get original autoID from snapshot meta err: %s", job.TableID, err.Error()) + return nil, nil, errors.Trace(err) } - // Call DDL RestoreTable - err = domain.GetDomain(e.ctx).DDL().RestoreTable(e.ctx, table.Meta(), job.SchemaID, autoID, job.ID, job.StartTS) - return errors.Trace(err) + for i := len(jobs) - 1; i > 0; i-- { + job = jobs[i] + if job.Type != model.ActionDropTable { + continue + } + // Check gc safe point for getting snapshot infoSchema. + err = gcutil.ValidateSnapshotWithGCSafePoint(job.StartTS, gcSafePoint) + if err != nil { + return nil, nil, errors.Trace(err) + } + // Get the snapshot infoSchema before drop table. + snapInfo, err := dom.GetSnapshotInfoSchema(job.StartTS) + if err != nil { + return nil, nil, errors.Trace(err) + } + // Get table meta from snapshot infoSchema. + table, ok := snapInfo.TableByID(job.TableID) + if !ok { + return nil, nil, infoschema.ErrTableNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", job.SchemaID), + fmt.Sprintf("(Table ID %d)", job.TableID), + ) + } + if table.Meta().Name == e.Table.Name { + tblInfo = table.Meta() + break + } + } + if tblInfo == nil { + return nil, nil, errors.Errorf("Can't found drop table: %v in ddl history jobs", e.Table.Name) + } + return job, tblInfo, nil } diff --git a/go.mod b/go.mod index 45ba9b40cc01a..3c62e7bbf1d0e 100644 --- a/go.mod +++ b/go.mod @@ -86,3 +86,5 @@ require ( sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4 sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) + +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f diff --git a/go.sum b/go.sum index c7358410e7861..71ccf064b4d9b 100644 --- a/go.sum +++ b/go.sum @@ -22,6 +22,8 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= +github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f h1:jWMAlv+kw3dIB02ubt2HS/jHfptOwWyKLhBjvsGCjLY= +github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= diff --git a/planner/core/common_plans.go b/planner/core/common_plans.go index 99d5c9d642c46..37adc390b402c 100644 --- a/planner/core/common_plans.go +++ b/planner/core/common_plans.go @@ -87,7 +87,9 @@ type RecoverIndex struct { // RestoreTable is used for recover deleted files by mistake. type RestoreTable struct { baseSchemaProducer - JobID int64 + JobID int64 + Table *ast.TableName + JobNum int64 } // CleanupIndex is used to delete dangling index data. diff --git a/planner/core/planbuilder.go b/planner/core/planbuilder.go index 7c7daaa57251f..b4e5839dad69a 100644 --- a/planner/core/planbuilder.go +++ b/planner/core/planbuilder.go @@ -551,7 +551,13 @@ func (b *PlanBuilder) buildAdmin(as *ast.AdminStmt) (Plan, error) { p.SetSchema(buildShowSlowSchema()) ret = p case ast.AdminRestoreTable: - ret = &RestoreTable{JobID: as.JobIDs[0]} + if len(as.JobIDs) > 0 { + ret = &RestoreTable{JobID: as.JobIDs[0]} + } else if len(as.Tables) > 0 { + ret = &RestoreTable{Table: as.Tables[0], JobNum: as.JobNumber} + } else { + return nil, ErrUnsupportedType.GenWithStack("Unsupported ast.AdminStmt(%T) for buildAdmin", as) + } default: return nil, ErrUnsupportedType.GenWithStack("Unsupported ast.AdminStmt(%T) for buildAdmin", as) } diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index 4a29bf7312a63..3bb8befb89ce8 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -57,22 +57,48 @@ func EnableGC(ctx sessionctx.Context) error { // ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. func ValidateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { - sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" - rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + safePointString, err := GetGCSafePoint(ctx) if err != nil { return errors.Trace(err) } - if len(rows) != 1 { - return errors.New("can not get 'tikv_gc_safe_point'") - } - safePointString := rows[0].GetString(0) safePointTime, err := util.CompatibleParseGCTime(safePointString) if err != nil { return errors.Trace(err) } safePointTS := variable.GoTimeToTS(safePointTime) + if err != nil { + return errors.Trace(err) + } if safePointTS > snapshotTS { return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) } return nil } + +// ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. +func ValidateSnapshotWithGCSafePoint(snapshotTS uint64, safePointString string) error { + safePointTime, err := util.CompatibleParseGCTime(safePointString) + if err != nil { + return errors.Trace(err) + } + gcSafePointTS := variable.GoTimeToTS(safePointTime) + if err != nil { + return errors.Trace(err) + } + if gcSafePointTS > snapshotTS { + return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) + } + return nil +} + +func GetGCSafePoint(ctx sessionctx.Context) (string, error) { + sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" + rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) + if err != nil { + return "", errors.Trace(err) + } + if len(rows) != 1 { + return "", errors.New("can not get 'tikv_gc_safe_point'") + } + return rows[0].GetString(0), nil +} From 18d4a5f0eefb229bd5a2868db5cdcfdaec13c6c4 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 12 Jan 2019 22:46:07 +0800 Subject: [PATCH 33/48] add syntax: restore table [table_name] --- ddl/db_test.go | 5 ++--- util/gcutil/gcutil.go | 41 ++++++++++++++++------------------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index bc1abfe0cd2e7..066ca1395c0d8 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -39,7 +39,6 @@ import ( "github.com/pingcap/tidb/meta/autoid" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/sessionctx" - "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/store/mockstore" "github.com/pingcap/tidb/store/mockstore/mocktikv" "github.com/pingcap/tidb/table" @@ -2358,7 +2357,7 @@ func (s *testDBSuite) TestRestoreTable(c *C) { tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) + c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) // set gc safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) @@ -2459,7 +2458,7 @@ func (s *testDBSuite) TestRestoreTableByTableName(c *C) { tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec("admin restore table t_recover") c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, variable.ErrSnapshotTooOld.GenWithStackByArgs(timeAfterDrop).Error()) + c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) // set gc safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index 3bb8befb89ce8..4c4053982f883 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -16,6 +16,7 @@ package gcutil import ( "fmt" "github.com/pingcap/errors" + "github.com/pingcap/parser/model" "github.com/pingcap/tidb/sessionctx" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util" @@ -57,48 +58,38 @@ func EnableGC(ctx sessionctx.Context) error { // ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. func ValidateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { - safePointString, err := GetGCSafePoint(ctx) - if err != nil { - return errors.Trace(err) - } - safePointTime, err := util.CompatibleParseGCTime(safePointString) - if err != nil { - return errors.Trace(err) - } - safePointTS := variable.GoTimeToTS(safePointTime) + safePointTS, err := GetGCSafePoint(ctx) if err != nil { return errors.Trace(err) } if safePointTS > snapshotTS { - return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) + return variable.ErrSnapshotTooOld.GenWithStackByArgs(model.TSConvert2Time(safePointTS).String()) } return nil } // ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. -func ValidateSnapshotWithGCSafePoint(snapshotTS uint64, safePointString string) error { - safePointTime, err := util.CompatibleParseGCTime(safePointString) - if err != nil { - return errors.Trace(err) - } - gcSafePointTS := variable.GoTimeToTS(safePointTime) - if err != nil { - return errors.Trace(err) - } - if gcSafePointTS > snapshotTS { - return variable.ErrSnapshotTooOld.GenWithStackByArgs(safePointString) +func ValidateSnapshotWithGCSafePoint(snapshotTS, safePointTS uint64) error { + if safePointTS > snapshotTS { + return variable.ErrSnapshotTooOld.GenWithStackByArgs(model.TSConvert2Time(safePointTS).String()) } return nil } -func GetGCSafePoint(ctx sessionctx.Context) (string, error) { +func GetGCSafePoint(ctx sessionctx.Context) (uint64, error) { sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) if err != nil { - return "", errors.Trace(err) + return 0, errors.Trace(err) } if len(rows) != 1 { - return "", errors.New("can not get 'tikv_gc_safe_point'") + return 0, errors.New("can not get 'tikv_gc_safe_point'") + } + safePointString := rows[0].GetString(0) + safePointTime, err := util.CompatibleParseGCTime(safePointString) + if err != nil { + return 0, errors.Trace(err) } - return rows[0].GetString(0), nil + ts := variable.GoTimeToTS(safePointTime) + return ts, nil } From 0ed20c5090f542abfc8da628408ad7e77944f530 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sat, 12 Jan 2019 23:49:55 +0800 Subject: [PATCH 34/48] check schema name --- executor/ddl.go | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/executor/ddl.go b/executor/ddl.go index 3eeab255cd6a5..69ca1df64d877 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -26,6 +26,7 @@ import ( "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/meta" + "github.com/pingcap/tidb/planner/core" "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/admin" @@ -386,6 +387,14 @@ func getRestoreTableByTableName(e *RestoreTableExec, t *meta.Meta, dom *domain.D if err != nil { return nil, nil, errors.Trace(err) } + schemaName := e.Table.Schema.L + if schemaName == "" { + schemaName = e.ctx.GetSessionVars().CurrentDB + } + if schemaName == "" { + return nil, nil, errors.Trace(core.ErrNoDB) + } + // TODO: only search recent `e.JobNum` ddl jobs. for i := len(jobs) - 1; i > 0; i-- { job = jobs[i] if job.Type != model.ActionDropTable { @@ -410,8 +419,16 @@ func getRestoreTableByTableName(e *RestoreTableExec, t *meta.Meta, dom *domain.D ) } if table.Meta().Name == e.Table.Name { - tblInfo = table.Meta() - break + schema, ok := dom.InfoSchema().SchemaByID(job.SchemaID) + if !ok { + return nil, nil, errors.Trace(infoschema.ErrDatabaseNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", job.SchemaID), + )) + } + if schema.Name.L == schemaName { + tblInfo = table.Meta() + break + } } } if tblInfo == nil { From 538b23a4555ef3d6a845784957f950b3b4f79dec Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Sun, 13 Jan 2019 14:36:35 +0800 Subject: [PATCH 35/48] update comment --- ddl/db_test.go | 2 +- util/gcutil/gcutil.go | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 066ca1395c0d8..b5d2f2a883dbc 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -2300,7 +2300,7 @@ func (s *testDBSuite) TestAddColumn2(c *C) { s.tk.MustQuery("select a,b,_tidb_rowid from t2").Check(testkit.Rows("1 3 2")) } -func (s *testDBSuite) TestRestoreTable(c *C) { +func (s *testDBSuite) TestRestoreTableByJobID(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index 4c4053982f883..f7605553ab6e7 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -68,7 +68,7 @@ func ValidateSnapshot(ctx sessionctx.Context, snapshotTS uint64) error { return nil } -// ValidateSnapshot checks that the newly set snapshot time is after GC safe point time. +// ValidateSnapshotWithGCSafePoint checks that the newly set snapshot time is after GC safe point time. func ValidateSnapshotWithGCSafePoint(snapshotTS, safePointTS uint64) error { if safePointTS > snapshotTS { return variable.ErrSnapshotTooOld.GenWithStackByArgs(model.TSConvert2Time(safePointTS).String()) @@ -76,6 +76,7 @@ func ValidateSnapshotWithGCSafePoint(snapshotTS, safePointTS uint64) error { return nil } +// GetGCSafePoint loads gc safe point time from mysql.tidb. func GetGCSafePoint(ctx sessionctx.Context) (uint64, error) { sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) From 157061dbc8a81f3e8bc1622e50e4dca83dcffdbb Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 11:05:23 +0800 Subject: [PATCH 36/48] update parser and address comment --- go.mod | 2 +- go.sum | 2 ++ util/gcutil/gcutil.go | 21 +++++++++++---------- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index 3c62e7bbf1d0e..6bdd65f9f9591 100644 --- a/go.mod +++ b/go.mod @@ -87,4 +87,4 @@ require ( sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970 diff --git a/go.sum b/go.sum index 71ccf064b4d9b..24a7aa2d71b46 100644 --- a/go.sum +++ b/go.sum @@ -24,6 +24,8 @@ github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbp github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f h1:jWMAlv+kw3dIB02ubt2HS/jHfptOwWyKLhBjvsGCjLY= github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= +github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970 h1:IpdAAjZr/OlsVcuO3UNcFSW40dMyYpl2lQElKsjR4jw= +github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index f7605553ab6e7..1549bf333f0a4 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -23,9 +23,16 @@ import ( "github.com/pingcap/tidb/util/sqlexec" ) +const ( + selectVariableValueSQL = `SELECT HIGH_PRIORITY variable_value FROM mysql.tidb WHERE variable_name='%s'` + insertVariableValueSQL = `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') + ON DUPLICATE KEY + UPDATE variable_value = '%[2]s', comment = '%[3]s'` +) + // CheckGCEnable is use to check whether gc is enable. func CheckGCEnable(ctx sessionctx.Context) (enable bool, err error) { - sql := fmt.Sprintf(`SELECT HIGH_PRIORITY (variable_value) FROM mysql.tidb WHERE variable_name='%s' FOR UPDATE`, "tikv_gc_enable") + sql := fmt.Sprintf(selectVariableValueSQL, "tikv_gc_enable") rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) if err != nil { return false, errors.Trace(err) @@ -38,20 +45,14 @@ func CheckGCEnable(ctx sessionctx.Context) (enable bool, err error) { // DisableGC will disable gc enable variable. func DisableGC(ctx sessionctx.Context) error { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') - ON DUPLICATE KEY - UPDATE variable_value = '%[2]s', comment = '%[3]s'`, - "tikv_gc_enable", "false", "Current GC enable status") + sql := fmt.Sprintf(insertVariableValueSQL, "tikv_gc_enable", "false", "Current GC enable status") _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) return errors.Trace(err) } // EnableGC will enable gc enable variable. func EnableGC(ctx sessionctx.Context) error { - sql := fmt.Sprintf(`INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('%[1]s', '%[2]s', '%[3]s') - ON DUPLICATE KEY - UPDATE variable_value = '%[2]s', comment = '%[3]s'`, - "tikv_gc_enable", "true", "Current GC enable status") + sql := fmt.Sprintf(insertVariableValueSQL, "tikv_gc_enable", "true", "Current GC enable status") _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) return errors.Trace(err) } @@ -78,7 +79,7 @@ func ValidateSnapshotWithGCSafePoint(snapshotTS, safePointTS uint64) error { // GetGCSafePoint loads gc safe point time from mysql.tidb. func GetGCSafePoint(ctx sessionctx.Context) (uint64, error) { - sql := "SELECT variable_value FROM mysql.tidb WHERE variable_name = 'tikv_gc_safe_point'" + sql := fmt.Sprintf(selectVariableValueSQL, "tikv_gc_safe_point") rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) if err != nil { return 0, errors.Trace(err) From 5580550c20c2a21822f5fbdde4251b46279b62ff Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 11:27:33 +0800 Subject: [PATCH 37/48] add check in preprocesst --- go.mod | 2 +- go.sum | 2 ++ planner/core/preprocess.go | 2 ++ 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 6bdd65f9f9591..aa134be73dd96 100644 --- a/go.mod +++ b/go.mod @@ -87,4 +87,4 @@ require ( sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970 +replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d diff --git a/go.sum b/go.sum index 24a7aa2d71b46..08ddc75452ecc 100644 --- a/go.sum +++ b/go.sum @@ -26,6 +26,8 @@ github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f h1:jWMAlv+kw3dIB github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970 h1:IpdAAjZr/OlsVcuO3UNcFSW40dMyYpl2lQElKsjR4jw= github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= +github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d h1:3ai8yKL/fsn/bZI8HHzDjDvZai0EN16mds+z6GZZWmk= +github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index a89d058ddcb94..8b81ff195caf9 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -85,6 +85,8 @@ func (p *preprocessor) Enter(in ast.Node) (out ast.Node, skipChildren bool) { return in, true case *ast.Join: p.checkNonUniqTableAlias(node) + case *ast.AdminStmt: + return in, node.Tp == ast.AdminRestoreTable default: p.parentIsJoin = false } From a48119c8a0f102f4c719b7facadafe6be9add7cc Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 12:06:04 +0800 Subject: [PATCH 38/48] add comment --- planner/core/preprocess.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/planner/core/preprocess.go b/planner/core/preprocess.go index 8b81ff195caf9..b3422ffb40e10 100644 --- a/planner/core/preprocess.go +++ b/planner/core/preprocess.go @@ -86,6 +86,9 @@ func (p *preprocessor) Enter(in ast.Node) (out ast.Node, skipChildren bool) { case *ast.Join: p.checkNonUniqTableAlias(node) case *ast.AdminStmt: + // The specified table in admin restore syntax maybe already been dropped. + // So skip check table name here, otherwise, admin restore table [table_name] syntax will return + // table not exists error. But admin restore is use to restore the dropped table. So skip children here. return in, node.Tp == ast.AdminRestoreTable default: p.parentIsJoin = false From 286c419f255b7b80e2eb9ab20ec89056988439b7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 19:00:18 +0800 Subject: [PATCH 39/48] update parser go.mod --- go.mod | 4 +--- go.sum | 10 ++-------- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/go.mod b/go.mod index aa134be73dd96..e1684976606c2 100644 --- a/go.mod +++ b/go.mod @@ -48,7 +48,7 @@ require ( github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3 github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c - github.com/pingcap/parser v0.0.0-20190108104142-35fab0be7fca + github.com/pingcap/parser v0.0.0-20190114105451-005df5698910 github.com/pingcap/pd v2.1.0-rc.4+incompatible github.com/pingcap/tidb-tools v2.1.3-0.20190104033906-883b07a04a73+incompatible github.com/pingcap/tipb v0.0.0-20181012112600-11e33c750323 @@ -86,5 +86,3 @@ require ( sourcegraph.com/sourcegraph/appdash v0.0.0-20180531100431-4c381bd170b4 sourcegraph.com/sourcegraph/appdash-data v0.0.0-20151005221446-73f23eafcf67 ) - -replace github.com/pingcap/parser => github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d diff --git a/go.sum b/go.sum index 08ddc75452ecc..47238b320c4ad 100644 --- a/go.sum +++ b/go.sum @@ -22,12 +22,6 @@ github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142 h1:3jFq2xL4ZajGK github.com/coreos/go-systemd v0.0.0-20181031085051-9002847aa142/go.mod h1:F5haX7vjVVG0kc13fIWeqUViNPyEJxv/OmvnBo0Yme4= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f h1:lBNOc5arjvs8E5mO2tbpBpLoyyu8B6e44T7hJy6potg= github.com/coreos/pkg v0.0.0-20180928190104-399ea9e2e55f/go.mod h1:E3G3o1h8I7cfcXa63jLwjI0eiQQMgzzUDFVpN/nH/eA= -github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f h1:jWMAlv+kw3dIB02ubt2HS/jHfptOwWyKLhBjvsGCjLY= -github.com/crazycs520/parser v0.0.0-20190112125519-01bbcdca7f4f/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= -github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970 h1:IpdAAjZr/OlsVcuO3UNcFSW40dMyYpl2lQElKsjR4jw= -github.com/crazycs520/parser v0.0.0-20190114024014-c751fe5de970/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= -github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d h1:3ai8yKL/fsn/bZI8HHzDjDvZai0EN16mds+z6GZZWmk= -github.com/crazycs520/parser v0.0.0-20190114032113-044c1033c14d/go.mod h1:xLjI+gnWYexq011WPMEvCNS8rFM9qe1vdojIEzSKPuc= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7 h1:y+DH9ARrWiiNBV+6waYP2IPcsRbxdU1qsnycPfShF4c= github.com/cznic/mathutil v0.0.0-20181021201202-eba54fb065b7/go.mod h1:e6NPNENfs9mPDVNRekM7lKScauxd5kXTr1Mfyig6TDM= github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 h1:hxuZop6tSoOi0sxFzoGGYdRqNrPubyaIf9KoBG9tPiE= @@ -116,8 +110,8 @@ github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e h1:P73/4dPCL96rG github.com/pingcap/goleveldb v0.0.0-20171020122428-b9ff6c35079e/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c h1:Qf5St5XGwKgKQLar9lEXoeO0hJMVaFBj3JqvFguWtVg= github.com/pingcap/kvproto v0.0.0-20181203065228-c14302da291c/go.mod h1:Ja9XPjot9q4/3JyCZodnWDGNXt4pKemhIYCvVJM7P24= -github.com/pingcap/parser v0.0.0-20190108104142-35fab0be7fca h1:BIk063GpkHOJzR4Bp9I8xfsjp3nSvElvhIUjDgmInoo= -github.com/pingcap/parser v0.0.0-20190108104142-35fab0be7fca/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA= +github.com/pingcap/parser v0.0.0-20190114105451-005df5698910 h1:3kybw5XEJIcAkOQ1t8UuohQu+O3ndC/mRsJZFOEi83U= +github.com/pingcap/parser v0.0.0-20190114105451-005df5698910/go.mod h1:1FNvfp9+J0wvc4kl8eGNh7Rqrxveg15jJoWo/a0uHwA= github.com/pingcap/pd v2.1.0-rc.4+incompatible h1:/buwGk04aHO5odk/+O8ZOXGs4qkUjYTJ2UpCJXna8NE= github.com/pingcap/pd v2.1.0-rc.4+incompatible/go.mod h1:nD3+EoYes4+aNNODO99ES59V83MZSI+dFbhyr667a0E= github.com/pingcap/tidb-tools v2.1.3-0.20190104033906-883b07a04a73+incompatible h1:Ba48wwPwPq5hd1kkQpgua49dqB5cthC2zXVo7fUUDec= From 378aef6fe87c9d1dcdf90bd3410011a1f7ba2cd8 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 20:51:24 +0800 Subject: [PATCH 40/48] address comment --- ddl/table.go | 16 ++++++++-------- infoschema/builder.go | 2 +- kv/txn.go | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/ddl/table.go b/ddl/table.go index 775d9b6851ffb..077dfee2e624e 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -179,20 +179,20 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // Restore table divide into 2 steps: // 1. Check gc enable status, to decided whether enable gc after restore table. - // 1. Why not disable gc before put the job to ddl job queue? + // a. Why not disable gc before put the job to ddl job queue? // Think about concurrency problem. If a restore job-1 is doing and already disabled GC, // then, another restore table job-2 check gc enable will get disable before into the job queue. // then, after restore table job-2 finished, the gc will be disabled. - // 2. Why split into 2 steps? 1 step also can finish this job: check gc -> disable gc -> restore table -> finish job. + // b. Why split into 2 steps? 1 step also can finish this job: check gc -> disable gc -> restore table -> finish job. // What if the transaction commit failed? then, the job will retry, but the gc already disabled when first running. // So, after this job retry succeed, the gc will be disabled. // 2. Do restore table job. - // 1. Check whether gc enabled, if enabled, disable gc first. - // 2. Check gc safe point. If drop table time if after safe point time, then can do restore. + // a. Check whether gc enabled, if enabled, disable gc first. + // b. Check gc safe point. If drop table time if after safe point time, then can do restore. // otherwise, can't restore table, because the records of the table may already delete by gc. - // 3. Remove gc task of the table from gc_delete_range table. - // 4. Create table and rebase table auto ID. - // 5. Finish. + // c. Remove gc task of the table from gc_delete_range table. + // d. Create table and rebase table auto ID. + // e. Finish. switch tblInfo.State { case model.StateNone: // none -> write only @@ -252,7 +252,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in return ver, nil } -// mockRestoreTableCommitErrOnce use to make sure `mockRestoreTableCommitErr` only mock error once. +// mockRestoreTableCommitErrOnce uses to make sure `mockRestoreTableCommitErr` only mock error once. var mockRestoreTableCommitErrOnce = true func enableGC(w *worker) error { diff --git a/infoschema/builder.go b/infoschema/builder.go index 10a1dadf40974..b9e1d5b3ba8a0 100644 --- a/infoschema/builder.go +++ b/infoschema/builder.go @@ -51,7 +51,7 @@ func (b *Builder) ApplyDiff(m *meta.Meta, diff *model.SchemaDiff) ([]int64, erro var oldTableID, newTableID int64 tblIDs := make([]int64, 0, 2) switch diff.Type { - case model.ActionCreateTable: + case model.ActionCreateTable, model.ActionRestoreTable: newTableID = diff.TableID tblIDs = append(tblIDs, newTableID) case model.ActionDropTable, model.ActionDropView: diff --git a/kv/txn.go b/kv/txn.go index ff35490746e77..00ee680049ad4 100644 --- a/kv/txn.go +++ b/kv/txn.go @@ -125,7 +125,7 @@ func BatchGetValues(txn Transaction, keys []Key) (map[string][]byte, error) { return storageValues, nil } -// mockCommitErrorEnable use to enable `mockCommitError` and only mock error once. +// mockCommitErrorEnable uses to enable `mockCommitError` and only mock error once. var mockCommitErrorEnable = int64(0) // MockCommitErrorEnable exports for gofail testing. From c2cd61e7ece0303d9c300d4f5c1affa3a1da4aa5 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Mon, 14 Jan 2019 20:57:57 +0800 Subject: [PATCH 41/48] address comment --- meta/meta.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meta/meta.go b/meta/meta.go index 0e546b1a7d756..59751b45ce7a4 100644 --- a/meta/meta.go +++ b/meta/meta.go @@ -297,7 +297,7 @@ func (m *Meta) CreateTable(dbID int64, tableInfo *model.TableInfo) error { } // CreateTableAndSetAutoID creates a table with tableInfo in database, -// and rebase the table autoID. +// and rebases the table autoID. func (m *Meta) CreateTableAndSetAutoID(dbID int64, tableInfo *model.TableInfo, autoID int64) error { err := m.CreateTable(dbID, tableInfo) if err != nil { From 806b7ad6a08d278478d7fcf9a3165f8fa02f5acd Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Tue, 15 Jan 2019 18:43:21 +0800 Subject: [PATCH 42/48] add check before job enqueue --- executor/ddl.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/executor/ddl.go b/executor/ddl.go index 32eca15436c5b..30371bc1cc6fd 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -328,6 +328,18 @@ func (e *RestoreTableExec) Next(ctx context.Context, req *chunk.RecordBatch) err if err != nil { return errors.Trace(err) } + // Check schema exist. + schema, ok := dom.InfoSchema().SchemaByID(job.SchemaID) + if !ok { + return errors.Trace(infoschema.ErrDatabaseNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", job.SchemaID), + )) + } + // Check not exist table with same name. + if ok := dom.InfoSchema().TableExists(schema.Name, tblInfo.Name); ok { + return infoschema.ErrTableExists.GenWithStackByArgs(tblInfo.Name) + } + // Get table original autoID before table drop. m, err := dom.GetSnapshotMeta(job.StartTS) if err != nil { From dde564759f897eb501ff43b602f162a36feee5ee Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 15 Jan 2019 19:31:35 +0800 Subject: [PATCH 43/48] move test to serial execute --- ddl/db_test.go | 204 -------------------------------------------- ddl/serial_test.go | 207 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 206 insertions(+), 205 deletions(-) diff --git a/ddl/db_test.go b/ddl/db_test.go index 1c03a6d017598..f669f1c6d316e 100644 --- a/ddl/db_test.go +++ b/ddl/db_test.go @@ -46,7 +46,6 @@ import ( "github.com/pingcap/tidb/tablecodec" "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/admin" - "github.com/pingcap/tidb/util/gcutil" "github.com/pingcap/tidb/util/mock" "github.com/pingcap/tidb/util/schemautil" "github.com/pingcap/tidb/util/testkit" @@ -2324,206 +2323,3 @@ func (s *testDBSuite) TestAddColumn2(c *C) { re.Check(testkit.Rows("1 2")) s.tk.MustQuery("select a,b,_tidb_rowid from t2").Check(testkit.Rows("1 3 2")) } - -func (s *testDBSuite) TestRestoreTableByJobID(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("create database if not exists test_restore") - tk.MustExec("use test_restore") - tk.MustExec("drop table if exists t_recover") - tk.MustExec("create table t_recover (a int);") - defer func(originGC bool) { - if originGC { - ddl.EmulatorGCEnable() - } else { - ddl.EmulatorGCDisable() - } - }(ddl.IsEmulatorGCEnable()) - - // disable emulator GC. - // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. - ddl.EmulatorGCDisable() - gcTimeFormat := "20060102-15:04:05 -0700 MST" - timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) - timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) - safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') - ON DUPLICATE KEY - UPDATE variable_value = '%[1]s'` - // clear gc variables first. - tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") - - tk.MustExec("insert into t_recover values (1),(2),(3)") - tk.MustExec("drop table t_recover") - - rs, err := tk.Exec("admin show ddl jobs") - c.Assert(err, IsNil) - rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) - c.Assert(err, IsNil) - row := rows[0] - c.Assert(row.GetString(1), Equals, "test_restore") - c.Assert(row.GetString(3), Equals, "drop table") - jobID := row.GetInt64(0) - - // if gc safe point is not exists in mysql.tidb - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - - // if gc enable is not exists in mysql.tidb - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") - - err = gcutil.EnableGC(tk.Se) - c.Assert(err, IsNil) - - // recover job is before gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err, NotNil) - c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) - - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - // if there is a new table with the same name, should return failed. - tk.MustExec("create table t_recover (a int);") - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) - c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) - - // drop the new table with the same name, then restore table. - tk.MustExec("drop table t_recover") - - // do restore table. - tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (4),(5),(6)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) - - // restore table by none exits job. - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) - c.Assert(err, NotNil) - - // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. - err = gcutil.DisableGC(tk.Se) - c.Assert(err, IsNil) - - tk.MustExec("delete from t_recover where a > 1") - tk.MustExec("drop table t_recover") - rs, err = tk.Exec("admin show ddl jobs") - c.Assert(err, IsNil) - rows, err = session.GetRows4Test(context.Background(), tk.Se, rs) - c.Assert(err, IsNil) - row = rows[0] - c.Assert(row.GetString(1), Equals, "test_restore") - c.Assert(row.GetString(3), Equals, "drop table") - jobID = row.GetInt64(0) - - tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (7),(8),(9)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) - - gcEnable, err := gcutil.CheckGCEnable(tk.Se) - c.Assert(err, IsNil) - c.Assert(gcEnable, Equals, false) -} - -func (s *testDBSuite) TestRestoreTableByTableName(c *C) { - tk := testkit.NewTestKit(c, s.store) - tk.MustExec("create database if not exists test_restore") - tk.MustExec("use test_restore") - tk.MustExec("drop table if exists t_recover, t_recover2") - tk.MustExec("create table t_recover (a int);") - defer func(originGC bool) { - if originGC { - ddl.EmulatorGCEnable() - } else { - ddl.EmulatorGCDisable() - } - }(ddl.IsEmulatorGCEnable()) - - // disable emulator GC. - // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. - ddl.EmulatorGCDisable() - gcTimeFormat := "20060102-15:04:05 -0700 MST" - timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) - timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) - safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') - ON DUPLICATE KEY - UPDATE variable_value = '%[1]s'` - // clear gc variables first. - tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") - - tk.MustExec("insert into t_recover values (1),(2),(3)") - tk.MustExec("drop table t_recover") - - // if gc safe point is not exists in mysql.tidb - _, err := tk.Exec("admin restore table t_recover") - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - - // if gc enable is not exists in mysql.tidb - _, err = tk.Exec("admin restore table t_recover") - c.Assert(err, NotNil) - c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") - - err = gcutil.EnableGC(tk.Se) - c.Assert(err, IsNil) - - // recover job is before gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) - _, err = tk.Exec("admin restore table t_recover") - c.Assert(err, NotNil) - c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) - - // set gc safe point - tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - // if there is a new table with the same name, should return failed. - tk.MustExec("create table t_recover (a int);") - _, err = tk.Exec("admin restore table t_recover") - c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) - - // drop the new table with the same name, then restore table. - tk.MustExec("rename table t_recover to t_recover2") - - // do restore table. - tk.MustExec("admin restore table t_recover") - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (4),(5),(6)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) - - // restore table by none exits job. - _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) - c.Assert(err, NotNil) - - // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. - err = gcutil.DisableGC(tk.Se) - c.Assert(err, IsNil) - - tk.MustExec("delete from t_recover where a > 1") - tk.MustExec("drop table t_recover") - - tk.MustExec("admin restore table t_recover") - - // check recover table meta and data record. - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) - // check recover table autoID. - tk.MustExec("insert into t_recover values (7),(8),(9)") - tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) - - gcEnable, err := gcutil.CheckGCEnable(tk.Se) - c.Assert(err, IsNil) - c.Assert(gcEnable, Equals, false) -} diff --git a/ddl/serial_test.go b/ddl/serial_test.go index f906df2b19473..fa69ca2ca808c 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -16,6 +16,7 @@ package ddl_test import ( "context" "fmt" + "strings" "time" . "github.com/pingcap/check" @@ -24,6 +25,7 @@ import ( "github.com/pingcap/parser/model" "github.com/pingcap/tidb/ddl" "github.com/pingcap/tidb/domain" + "github.com/pingcap/tidb/infoschema" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/session" "github.com/pingcap/tidb/store/mockstore" @@ -126,7 +128,210 @@ func (s *testSerialSuite) TestCancelAddIndexPanic(c *C) { c.Assert(err.Error(), Equals, "[ddl:12]cancelled DDL job") } -func (s *testSerialSuite) TestRestoreTableFail(c *C) { +func (s *testSerialSuite) TestRestoreTableByJobID(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + // clear gc variables first. + tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + rs, err := tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err := session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row := rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID := row.GetInt64(0) + + // if gc safe point is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // if gc enable is not exists in mysql.tidb + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") + + err = gcutil.EnableGC(tk.Se) + c.Assert(err, IsNil) + + // recover job is before gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err, NotNil) + c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) + + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if there is a new table with the same name, should return failed. + tk.MustExec("create table t_recover (a int);") + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) + c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) + + // drop the new table with the same name, then restore table. + tk.MustExec("drop table t_recover") + + // do restore table. + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + + // restore table by none exits job. + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) + c.Assert(err, NotNil) + + // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + err = gcutil.DisableGC(tk.Se) + c.Assert(err, IsNil) + + tk.MustExec("delete from t_recover where a > 1") + tk.MustExec("drop table t_recover") + rs, err = tk.Exec("admin show ddl jobs") + c.Assert(err, IsNil) + rows, err = session.GetRows4Test(context.Background(), tk.Se, rs) + c.Assert(err, IsNil) + row = rows[0] + c.Assert(row.GetString(1), Equals, "test_restore") + c.Assert(row.GetString(3), Equals, "drop table") + jobID = row.GetInt64(0) + + tk.MustExec(fmt.Sprintf("admin restore table by job %d", jobID)) + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (7),(8),(9)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) + + gcEnable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(gcEnable, Equals, false) +} + +func (s *testSerialSuite) TestRestoreTableByTableName(c *C) { + tk := testkit.NewTestKit(c, s.store) + tk.MustExec("create database if not exists test_restore") + tk.MustExec("use test_restore") + tk.MustExec("drop table if exists t_recover, t_recover2") + tk.MustExec("create table t_recover (a int);") + defer func(originGC bool) { + if originGC { + ddl.EmulatorGCEnable() + } else { + ddl.EmulatorGCDisable() + } + }(ddl.IsEmulatorGCEnable()) + + // disable emulator GC. + // Otherwise emulator GC will delete table record as soon as possible after execute drop table ddl. + ddl.EmulatorGCDisable() + gcTimeFormat := "20060102-15:04:05 -0700 MST" + timeBeforeDrop := time.Now().Add(0 - time.Duration(48*60*60*time.Second)).Format(gcTimeFormat) + timeAfterDrop := time.Now().Add(time.Duration(48 * 60 * 60 * time.Second)).Format(gcTimeFormat) + safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') + ON DUPLICATE KEY + UPDATE variable_value = '%[1]s'` + // clear gc variables first. + tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") + + tk.MustExec("insert into t_recover values (1),(2),(3)") + tk.MustExec("drop table t_recover") + + // if gc safe point is not exists in mysql.tidb + _, err := tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + + // if gc enable is not exists in mysql.tidb + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") + + err = gcutil.EnableGC(tk.Se) + c.Assert(err, IsNil) + + // recover job is before gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err, NotNil) + c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) + + // set gc safe point + tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) + // if there is a new table with the same name, should return failed. + tk.MustExec("create table t_recover (a int);") + _, err = tk.Exec("admin restore table t_recover") + c.Assert(err.Error(), Equals, infoschema.ErrTableExists.GenWithStackByArgs("t_recover").Error()) + + // drop the new table with the same name, then restore table. + tk.MustExec("rename table t_recover to t_recover2") + + // do restore table. + tk.MustExec("admin restore table t_recover") + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (4),(5),(6)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + + // restore table by none exits job. + _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) + c.Assert(err, NotNil) + + // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + err = gcutil.DisableGC(tk.Se) + c.Assert(err, IsNil) + + tk.MustExec("delete from t_recover where a > 1") + tk.MustExec("drop table t_recover") + + tk.MustExec("admin restore table t_recover") + + // check recover table meta and data record. + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1")) + // check recover table autoID. + tk.MustExec("insert into t_recover values (7),(8),(9)") + tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "7", "8", "9")) + + gcEnable, err := gcutil.CheckGCEnable(tk.Se) + c.Assert(err, IsNil) + c.Assert(gcEnable, Equals, false) +} + +func (s *testSerialSuite) TestRestoreTableByJobIDFail(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") From b92ab5e90166e48a6c4cd50fc124abc8c5563c78 Mon Sep 17 00:00:00 2001 From: crazycs Date: Tue, 15 Jan 2019 19:54:34 +0800 Subject: [PATCH 44/48] fix ddl test --- executor/ddl_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/executor/ddl_test.go b/executor/ddl_test.go index 9f30dcde74636..a3c3806ad429e 100644 --- a/executor/ddl_test.go +++ b/executor/ddl_test.go @@ -577,7 +577,4 @@ func (s *testSuite3) TestSetDDLReorgBatchSize(c *C) { tk.MustExec("set @@global.tidb_ddl_reorg_batch_size = 1000") res = tk.MustQuery("select @@global.tidb_ddl_reorg_batch_size") res.Check(testkit.Rows("1000")) - - // If do not LoadDDLReorgVars, the local variable will be the last loaded value. - c.Assert(variable.GetDDLReorgBatchSize(), Equals, int32(100)) } From b3df2bf36c388a6293e18cd3c4b29b800b2a286b Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 16 Jan 2019 02:16:53 +0800 Subject: [PATCH 45/48] move check --- ddl/ddl_api.go | 13 +++++++++++++ executor/ddl.go | 12 ------------ util/gcutil/gcutil.go | 1 + 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index dc8c96b4da764..f344375c993aa 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1065,6 +1065,19 @@ func (d *ddl) CreateTable(ctx sessionctx.Context, s *ast.CreateTableStmt) (err e } func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, schemaID, autoID, dropJobID int64, snapshotTS uint64) (err error) { + is := d.GetInformationSchema(ctx) + // Check schema exist. + schema, ok := is.SchemaByID(schemaID) + if !ok { + return errors.Trace(infoschema.ErrDatabaseNotExists.GenWithStackByArgs( + fmt.Sprintf("(Schema ID %d)", schemaID), + )) + } + // Check not exist table with same name. + if ok := is.TableExists(schema.Name, tbInfo.Name); ok { + return infoschema.ErrTableExists.GenWithStackByArgs(tbInfo.Name) + } + tbInfo.State = model.StateNone job := &model.Job{ SchemaID: schemaID, diff --git a/executor/ddl.go b/executor/ddl.go index 30371bc1cc6fd..32eca15436c5b 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -328,18 +328,6 @@ func (e *RestoreTableExec) Next(ctx context.Context, req *chunk.RecordBatch) err if err != nil { return errors.Trace(err) } - // Check schema exist. - schema, ok := dom.InfoSchema().SchemaByID(job.SchemaID) - if !ok { - return errors.Trace(infoschema.ErrDatabaseNotExists.GenWithStackByArgs( - fmt.Sprintf("(Schema ID %d)", job.SchemaID), - )) - } - // Check not exist table with same name. - if ok := dom.InfoSchema().TableExists(schema.Name, tblInfo.Name); ok { - return infoschema.ErrTableExists.GenWithStackByArgs(tblInfo.Name) - } - // Get table original autoID before table drop. m, err := dom.GetSnapshotMeta(job.StartTS) if err != nil { diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index 1549bf333f0a4..0995daa6e4bc1 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -15,6 +15,7 @@ package gcutil import ( "fmt" + "github.com/pingcap/errors" "github.com/pingcap/parser/model" "github.com/pingcap/tidb/sessionctx" From 443b684db816502975de4798960a5f9a9b798b76 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 16 Jan 2019 03:04:56 +0800 Subject: [PATCH 46/48] address comment --- ddl/ddl_api.go | 1 + ddl/ddl_worker.go | 12 +++++++----- ddl/delete_range.go | 4 ++-- ddl/serial_test.go | 4 +++- ddl/table.go | 4 +++- domain/domain.go | 2 +- executor/ddl.go | 2 +- 7 files changed, 18 insertions(+), 11 deletions(-) diff --git a/ddl/ddl_api.go b/ddl/ddl_api.go index f344375c993aa..e541baf91617a 100644 --- a/ddl/ddl_api.go +++ b/ddl/ddl_api.go @@ -1087,6 +1087,7 @@ func (d *ddl) RestoreTable(ctx sessionctx.Context, tbInfo *model.TableInfo, sche Args: []interface{}{tbInfo, autoID, dropJobID, snapshotTS, restoreTableCheckFlagNone}, } err = d.doDDLJob(ctx, job) + err = d.callHookOnChanged(err) return errors.Trace(err) } diff --git a/ddl/ddl_worker.go b/ddl/ddl_worker.go index 6b3ff5bc4426b..09e95f4a6ceef 100644 --- a/ddl/ddl_worker.go +++ b/ddl/ddl_worker.go @@ -276,13 +276,15 @@ func (w *worker) finishDDLJob(t *meta.Meta, job *model.Job) (err error) { err = w.deleteRange(job) case model.ActionDropSchema, model.ActionDropTable, model.ActionTruncateTable, model.ActionDropIndex, model.ActionDropTablePartition, model.ActionTruncateTablePartition: err = w.deleteRange(job) - case model.ActionRestoreTable: - err = finishRestoreTable(w, t, job) - } - if err != nil { - return errors.Trace(err) } } + switch job.Type { + case model.ActionRestoreTable: + err = finishRestoreTable(w, t, job) + } + if err != nil { + return errors.Trace(err) + } _, err = t.DeQueueDDLJob() if err != nil { diff --git a/ddl/delete_range.go b/ddl/delete_range.go index 4ddab17ad6dd2..0c5f336355209 100644 --- a/ddl/delete_range.go +++ b/ddl/delete_range.go @@ -40,13 +40,13 @@ const ( ) // enableEmulatorGC means whether to enable emulator GC. The default is enable. -// In some unit test, we want to stop emulator GC, then wen can set enableEmulatorGC to 0. +// In some unit tests, we want to stop emulator GC, then wen can set enableEmulatorGC to 0. var emulatorGCEnable = int32(1) type delRangeManager interface { // addDelRangeJob add a DDL job into gc_delete_range table. addDelRangeJob(job *model.Job) error - // removeFromGCDeleteRange remove delete table job from gc_delete_range table by jobID and tableID. + // removeFromGCDeleteRange removes the deleting table job from gc_delete_range table by jobID and tableID. // It's use for recover the table that was mistakenly deleted. removeFromGCDeleteRange(jobID, tableID int64) error start() diff --git a/ddl/serial_test.go b/ddl/serial_test.go index fa69ca2ca808c..a804386a7f508 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -306,6 +306,8 @@ func (s *testSerialSuite) TestRestoreTableByTableName(c *C) { // check recover table autoID. tk.MustExec("insert into t_recover values (4),(5),(6)") tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) + // check rebase auto id. + tk.MustQuery("select a,_tidb_rowid from t_recover;").Check(testkit.Rows("1 1", "2 2", "3 3", "4 5001", "5 5002", "6 5003")) // restore table by none exits job. _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) @@ -400,7 +402,7 @@ func (s *testSerialSuite) TestRestoreTableByJobIDFail(c *C) { tk.MustQuery("select * from t_recover;").Check(testkit.Rows("1", "2", "3", "4", "5", "6")) } -func (s *testSerialSuite) TestRestoreTableFailByTableName(c *C) { +func (s *testSerialSuite) TestRestoreTableByTableNameFail(c *C) { tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_restore") tk.MustExec("use test_restore") diff --git a/ddl/table.go b/ddl/table.go index 077dfee2e624e..98c566aed664c 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -179,7 +179,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // Restore table divide into 2 steps: // 1. Check gc enable status, to decided whether enable gc after restore table. - // a. Why not disable gc before put the job to ddl job queue? + // a. Why not disable gc before put the job to DDL job queue? // Think about concurrency problem. If a restore job-1 is doing and already disabled GC, // then, another restore table job-2 check gc enable will get disable before into the job queue. // then, after restore table job-2 finished, the gc will be disabled. @@ -248,6 +248,8 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in // Finish this job. job.FinishTableJob(model.JobStateDone, model.StatePublic, ver, tblInfo) + default: + return ver, ErrInvalidTableState.GenWithStack("invalid restore table state %v", tblInfo.State) } return ver, nil } diff --git a/domain/domain.go b/domain/domain.go index 5b4266045dc1d..4c129f39aa931 100644 --- a/domain/domain.go +++ b/domain/domain.go @@ -255,7 +255,7 @@ func (do *Domain) GetSnapshotInfoSchema(snapshotTS uint64) (infoschema.InfoSchem return snapHandle.Get(), nil } -// GetSnapshotMeta get a new snapshot meta at startTS. +// GetSnapshotMeta gets a new snapshot meta at startTS. func (do *Domain) GetSnapshotMeta(startTS uint64) (*meta.Meta, error) { snapshot, err := do.store.GetSnapshot(kv.NewVersion(startTS)) if err != nil { diff --git a/executor/ddl.go b/executor/ddl.go index 32eca15436c5b..6bde09f0c6fcd 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -394,7 +394,7 @@ func getRestoreTableByTableName(e *RestoreTableExec, t *meta.Meta, dom *domain.D if schemaName == "" { return nil, nil, errors.Trace(core.ErrNoDB) } - // TODO: only search recent `e.JobNum` ddl jobs. + // TODO: only search recent `e.JobNum` DDL jobs. for i := len(jobs) - 1; i > 0; i-- { job = jobs[i] if job.Type != model.ActionDropTable { From 6946c08b3844b663ca1c7fcf599f82e76bed5446 Mon Sep 17 00:00:00 2001 From: crazycs Date: Wed, 16 Jan 2019 03:11:44 +0800 Subject: [PATCH 47/48] address comment --- ddl/delete_range.go | 2 +- ddl/serial_test.go | 32 ++++++++++++++++---------------- ddl/table.go | 26 +++++++++++++------------- executor/ddl.go | 4 ++-- util/gcutil/gcutil.go | 8 ++++---- 5 files changed, 36 insertions(+), 36 deletions(-) diff --git a/ddl/delete_range.go b/ddl/delete_range.go index 0c5f336355209..6fa032d0aaff1 100644 --- a/ddl/delete_range.go +++ b/ddl/delete_range.go @@ -153,7 +153,7 @@ func EmulatorGCDisable() { atomic.StoreInt32(&emulatorGCEnable, 0) } -// IsEmulatorGCEnable indicates whether emulator gc enabled. It exports for testing. +// IsEmulatorGCEnable indicates whether emulator GC enabled. It exports for testing. func IsEmulatorGCEnable() bool { return atomic.LoadInt32(&emulatorGCEnable) == 1 } diff --git a/ddl/serial_test.go b/ddl/serial_test.go index a804386a7f508..a7604a8d61165 100644 --- a/ddl/serial_test.go +++ b/ddl/serial_test.go @@ -151,7 +151,7 @@ func (s *testSerialSuite) TestRestoreTableByJobID(c *C) { safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') ON DUPLICATE KEY UPDATE variable_value = '%[1]s'` - // clear gc variables first. + // clear GC variables first. tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") tk.MustExec("insert into t_recover values (1),(2),(3)") @@ -166,14 +166,14 @@ func (s *testSerialSuite) TestRestoreTableByJobID(c *C) { c.Assert(row.GetString(3), Equals, "drop table") jobID := row.GetInt64(0) - // if gc safe point is not exists in mysql.tidb + // if GC safe point is not exists in mysql.tidb _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // set gc safe point + // set GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - // if gc enable is not exists in mysql.tidb + // if GC enable is not exists in mysql.tidb _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") @@ -181,13 +181,13 @@ func (s *testSerialSuite) TestRestoreTableByJobID(c *C) { err = gcutil.EnableGC(tk.Se) c.Assert(err, IsNil) - // recover job is before gc safe point + // recover job is before GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", jobID)) c.Assert(err, NotNil) c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) - // set gc safe point + // set GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) // if there is a new table with the same name, should return failed. tk.MustExec("create table t_recover (a int);") @@ -210,7 +210,7 @@ func (s *testSerialSuite) TestRestoreTableByJobID(c *C) { _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) c.Assert(err, NotNil) - // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + // Disable GC by manual first, then after recover table, the GC enable status should also be disabled. err = gcutil.DisableGC(tk.Se) c.Assert(err, IsNil) @@ -261,20 +261,20 @@ func (s *testSerialSuite) TestRestoreTableByTableName(c *C) { safePointSQL := `INSERT HIGH_PRIORITY INTO mysql.tidb VALUES ('tikv_gc_safe_point', '%[1]s', '') ON DUPLICATE KEY UPDATE variable_value = '%[1]s'` - // clear gc variables first. + // clear GC variables first. tk.MustExec("delete from mysql.tidb where variable_name in ( 'tikv_gc_safe_point','tikv_gc_enable' )") tk.MustExec("insert into t_recover values (1),(2),(3)") tk.MustExec("drop table t_recover") - // if gc safe point is not exists in mysql.tidb + // if GC safe point is not exists in mysql.tidb _, err := tk.Exec("admin restore table t_recover") c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "can not get 'tikv_gc_safe_point'") - // set gc safe point + // set GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) - // if gc enable is not exists in mysql.tidb + // if GC enable is not exists in mysql.tidb _, err = tk.Exec("admin restore table t_recover") c.Assert(err, NotNil) c.Assert(err.Error(), Equals, "[ddl:-1]can not get 'tikv_gc_enable'") @@ -282,13 +282,13 @@ func (s *testSerialSuite) TestRestoreTableByTableName(c *C) { err = gcutil.EnableGC(tk.Se) c.Assert(err, IsNil) - // recover job is before gc safe point + // recover job is before GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeAfterDrop)) _, err = tk.Exec("admin restore table t_recover") c.Assert(err, NotNil) c.Assert(strings.Contains(err.Error(), "snapshot is older than GC safe point"), Equals, true) - // set gc safe point + // set GC safe point tk.MustExec(fmt.Sprintf(safePointSQL, timeBeforeDrop)) // if there is a new table with the same name, should return failed. tk.MustExec("create table t_recover (a int);") @@ -313,7 +313,7 @@ func (s *testSerialSuite) TestRestoreTableByTableName(c *C) { _, err = tk.Exec(fmt.Sprintf("admin restore table by job %d", 10000000)) c.Assert(err, NotNil) - // Disable gc by manual first, then after recover table, the gc enable status should also be disabled. + // Disable GC by manual first, then after recover table, the GC enable status should also be disabled. err = gcutil.DisableGC(tk.Se) c.Assert(err, IsNil) @@ -390,7 +390,7 @@ func (s *testSerialSuite) TestRestoreTableByJobIDFail(c *C) { gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") gofail.Disable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr") - // make sure enable gc after restore table. + // make sure enable GC after restore table. enable, err := gcutil.CheckGCEnable(tk.Se) c.Assert(err, IsNil) c.Assert(enable, Equals, true) @@ -450,7 +450,7 @@ func (s *testSerialSuite) TestRestoreTableByTableNameFail(c *C) { gofail.Disable("github.com/pingcap/tidb/store/tikv/mockCommitError") gofail.Disable("github.com/pingcap/tidb/ddl/mockRestoreTableCommitErr") - // make sure enable gc after restore table. + // make sure enable GC after restore table. enable, err := gcutil.CheckGCEnable(tk.Se) c.Assert(err, IsNil) c.Assert(enable, Equals, true) diff --git a/ddl/table.go b/ddl/table.go index 98c566aed664c..a964c6100b554 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -165,7 +165,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in return ver, errors.Trace(err) } - // check gc and safe point + // check GC and safe point gcEnable, err := checkGCEnable(w) if err != nil { job.State = model.JobStateCancelled @@ -178,25 +178,25 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in } // Restore table divide into 2 steps: - // 1. Check gc enable status, to decided whether enable gc after restore table. - // a. Why not disable gc before put the job to DDL job queue? + // 1. Check GC enable status, to decided whether enable GC after restore table. + // a. Why not disable GC before put the job to DDL job queue? // Think about concurrency problem. If a restore job-1 is doing and already disabled GC, - // then, another restore table job-2 check gc enable will get disable before into the job queue. - // then, after restore table job-2 finished, the gc will be disabled. - // b. Why split into 2 steps? 1 step also can finish this job: check gc -> disable gc -> restore table -> finish job. - // What if the transaction commit failed? then, the job will retry, but the gc already disabled when first running. - // So, after this job retry succeed, the gc will be disabled. + // then, another restore table job-2 check GC enable will get disable before into the job queue. + // then, after restore table job-2 finished, the GC will be disabled. + // b. Why split into 2 steps? 1 step also can finish this job: check GC -> disable GC -> restore table -> finish job. + // What if the transaction commit failed? then, the job will retry, but the GC already disabled when first running. + // So, after this job retry succeed, the GC will be disabled. // 2. Do restore table job. - // a. Check whether gc enabled, if enabled, disable gc first. - // b. Check gc safe point. If drop table time if after safe point time, then can do restore. + // a. Check whether GC enabled, if enabled, disable GC first. + // b. Check GC safe point. If drop table time if after safe point time, then can do restore. // otherwise, can't restore table, because the records of the table may already delete by gc. - // c. Remove gc task of the table from gc_delete_range table. + // c. Remove GC task of the table from gc_delete_range table. // d. Create table and rebase table auto ID. // e. Finish. switch tblInfo.State { case model.StateNone: // none -> write only - // check gc enable and update flag. + // check GC enable and update flag. if gcEnable { job.Args[len(job.Args)-1] = restoreTableCheckFlagEnableGC } else { @@ -216,7 +216,7 @@ func (w *worker) onRestoreTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver in return ver, errors.Errorf("disable gc failed, try again later. err: %v", err) } } - // check gc safe point + // check GC safe point err = checkSafePoint(w, snapshotTS) if err != nil { job.State = model.JobStateCancelled diff --git a/executor/ddl.go b/executor/ddl.go index 6bde09f0c6fcd..17314e072e3b8 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -354,7 +354,7 @@ func getRestoreTableByJobID(e *RestoreTableExec, t *meta.Meta, dom *domain.Domai return nil, nil, errors.Errorf("Job %v type is %v, not drop table", job.ID, job.Type) } - // Check gc safe point for getting snapshot infoSchema. + // Check GC safe point for getting snapshot infoSchema. err = gcutil.ValidateSnapshot(e.ctx, job.StartTS) if err != nil { return nil, nil, errors.Trace(err) @@ -400,7 +400,7 @@ func getRestoreTableByTableName(e *RestoreTableExec, t *meta.Meta, dom *domain.D if job.Type != model.ActionDropTable { continue } - // Check gc safe point for getting snapshot infoSchema. + // Check GC safe point for getting snapshot infoSchema. err = gcutil.ValidateSnapshotWithGCSafePoint(job.StartTS, gcSafePoint) if err != nil { return nil, nil, errors.Trace(err) diff --git a/util/gcutil/gcutil.go b/util/gcutil/gcutil.go index 0995daa6e4bc1..986a4eb7533ea 100644 --- a/util/gcutil/gcutil.go +++ b/util/gcutil/gcutil.go @@ -31,7 +31,7 @@ const ( UPDATE variable_value = '%[2]s', comment = '%[3]s'` ) -// CheckGCEnable is use to check whether gc is enable. +// CheckGCEnable is use to check whether GC is enable. func CheckGCEnable(ctx sessionctx.Context) (enable bool, err error) { sql := fmt.Sprintf(selectVariableValueSQL, "tikv_gc_enable") rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) @@ -44,14 +44,14 @@ func CheckGCEnable(ctx sessionctx.Context) (enable bool, err error) { return rows[0].GetString(0) == "true", nil } -// DisableGC will disable gc enable variable. +// DisableGC will disable GC enable variable. func DisableGC(ctx sessionctx.Context) error { sql := fmt.Sprintf(insertVariableValueSQL, "tikv_gc_enable", "false", "Current GC enable status") _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) return errors.Trace(err) } -// EnableGC will enable gc enable variable. +// EnableGC will enable GC enable variable. func EnableGC(ctx sessionctx.Context) error { sql := fmt.Sprintf(insertVariableValueSQL, "tikv_gc_enable", "true", "Current GC enable status") _, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) @@ -78,7 +78,7 @@ func ValidateSnapshotWithGCSafePoint(snapshotTS, safePointTS uint64) error { return nil } -// GetGCSafePoint loads gc safe point time from mysql.tidb. +// GetGCSafePoint loads GC safe point time from mysql.tidb. func GetGCSafePoint(ctx sessionctx.Context) (uint64, error) { sql := fmt.Sprintf(selectVariableValueSQL, "tikv_gc_safe_point") rows, _, err := ctx.(sqlexec.RestrictedSQLExecutor).ExecRestrictedSQL(ctx, sql) From f72802d075ef61c422b7d528d92cf3e74a15d3c7 Mon Sep 17 00:00:00 2001 From: crazycs520 Date: Wed, 16 Jan 2019 12:39:10 +0800 Subject: [PATCH 48/48] address comment --- executor/ddl.go | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/executor/ddl.go b/executor/ddl.go index 17314e072e3b8..5f545a9bfe619 100644 --- a/executor/ddl.go +++ b/executor/ddl.go @@ -311,7 +311,30 @@ func (e *RestoreTableExec) Open(ctx context.Context) error { } // Next implements the Executor Open interface. -func (e *RestoreTableExec) Next(ctx context.Context, req *chunk.RecordBatch) error { +func (e *RestoreTableExec) Next(ctx context.Context, req *chunk.RecordBatch) (err error) { + // Should commit the previous transaction and create a new transaction. + if err = e.ctx.NewTxn(ctx); err != nil { + return errors.Trace(err) + } + defer func() { e.ctx.GetSessionVars().StmtCtx.IsDDLJobInQueue = false }() + + err = e.executeRestoreTable() + if err != nil { + return errors.Trace(err) + } + + dom := domain.GetDomain(e.ctx) + // Update InfoSchema in TxnCtx, so it will pass schema check. + is := dom.InfoSchema() + txnCtx := e.ctx.GetSessionVars().TxnCtx + txnCtx.InfoSchema = is + txnCtx.SchemaVersion = is.SchemaMetaVersion() + // DDL will force commit old transaction, after DDL, in transaction status should be false. + e.ctx.GetSessionVars().SetStatusFlag(mysql.ServerStatusInTrans, false) + return nil +} + +func (e *RestoreTableExec) executeRestoreTable() error { txn, err := e.ctx.Txn(true) if err != nil { return errors.Trace(err)