From a61225cda9ce05ccac7c24ff8e385c31f19684b8 Mon Sep 17 00:00:00 2001 From: Lonng Date: Sun, 5 May 2019 11:41:23 +0800 Subject: [PATCH] cherry-pick: replace gofail with the new failpoint implementation (#10327) --- Makefile | 44 +++++++++++------------ ddl/column.go | 23 ++++++------ ddl/fail_db_test.go | 38 ++++++++++++-------- ddl/fail_test.go | 6 ++-- ddl/index.go | 46 ++++++++++++------------ ddl/reorg.go | 12 +++---- ddl/table.go | 25 +++++++------ executor/aggregate.go | 1 + executor/executor_test.go | 33 +++++++++-------- executor/point_get.go | 34 ++++++++---------- executor/write_test.go | 6 ++-- go.mod | 5 +-- go.sum | 7 +--- owner/fail_test.go | 6 ++-- owner/manager.go | 24 ++++++++----- session/session_fail_test.go | 14 ++++---- session/txn.go | 30 +++++++++------- store/mockstore/mocktikv/rpc.go | 52 +++++++++++++++------------ store/tikv/2pc_fail_test.go | 32 +++++++++++------ store/tikv/coprocessor.go | 11 +++--- store/tikv/gcworker/gc_worker_test.go | 14 ++++---- store/tikv/region_request.go | 41 ++++++++++----------- store/tikv/sql_fail_test.go | 6 ++-- store/tikv/store_fail_test.go | 6 ++-- tablecodec/tablecodec_test.go | 8 +++-- util/codec/decimal.go | 10 +++--- 26 files changed, 287 insertions(+), 247 deletions(-) diff --git a/Makefile b/Makefile index c8651dc66bd86..189e66afc98ce 100644 --- a/Makefile +++ b/Makefile @@ -25,8 +25,8 @@ PACKAGES := $$($(PACKAGE_LIST)) PACKAGE_DIRECTORIES := $(PACKAGE_LIST) | sed 's|github.com/pingcap/$(PROJECT)/||' FILES := $$(find $$($(PACKAGE_DIRECTORIES)) -name "*.go" | grep -vE "vendor") -GOFAIL_ENABLE := $$(find $$PWD/ -type d | grep -vE "(\.git|_tools)" | xargs tools/bin/gofail enable) -GOFAIL_DISABLE := $$(find $$PWD/ -type d | grep -vE "(\.git|_tools)" | xargs tools/bin/gofail disable) +FAILPOINT_ENABLE := $$(find $$PWD/ -type d | grep -vE "(\.git|tools)" | xargs tools/bin/failpoint-ctl enable) +FAILPOINT_DISABLE := $$(find $$PWD/ -type d | grep -vE "(\.git|tools)" | xargs tools/bin/failpoint-ctl disable) LDFLAGS += -X "github.com/pingcap/parser/mysql.TiDBReleaseVersion=$(shell git describe --tags --dirty)" LDFLAGS += -X "github.com/pingcap/tidb/util/printer.TiDBBuildTS=$(shell date -u '+%Y-%m-%d %I:%M:%S')" @@ -112,34 +112,34 @@ test: checklist gotest explaintest explaintest: server @cd cmd/explaintest && ./run-tests.sh -s ../../bin/tidb-server -gotest: gofail-enable +gotest: failpoint-enable ifeq ("$(TRAVIS_COVERAGE)", "1") @echo "Running in TRAVIS_COVERAGE mode." @export log_level=error; \ go get github.com/go-playground/overalls go get github.com/mattn/goveralls - $(OVERALLS) -project=github.com/pingcap/tidb -covermode=count -ignore='.git,vendor,cmd,docs,LICENSES' || { $(GOFAIL_DISABLE); exit 1; } - $(GOVERALLS) -service=travis-ci -coverprofile=overalls.coverprofile || { $(GOFAIL_DISABLE); exit 1; } + $(OVERALLS) -project=github.com/pingcap/tidb -covermode=count -ignore='.git,vendor,cmd,docs,LICENSES' || { $(FAILPOINT_DISABLE); exit 1; } + $(GOVERALLS) -service=travis-ci -coverprofile=overalls.coverprofile || { $(FAILPOINT_DISABLE); exit 1; } else @echo "Running in native mode." @export log_level=error; \ - $(GOTEST) -ldflags '$(TEST_LDFLAGS)' -cover $(PACKAGES) || { $(GOFAIL_DISABLE); exit 1; } + $(GOTEST) -ldflags '$(TEST_LDFLAGS)' -cover $(PACKAGES) || { $(FAILPOINT_DISABLE); exit 1; } endif - @$(GOFAIL_DISABLE) + @$(FAILPOINT_DISABLE) -race: gofail-enable +race: failpoint-enable @export log_level=debug; \ - $(GOTEST) -timeout 20m -race $(PACKAGES) || { $(GOFAIL_DISABLE); exit 1; } - @$(GOFAIL_DISABLE) + $(GOTEST) -timeout 20m -race $(PACKAGES) || { $(FAILPOINT_DISABLE); exit 1; } + @$(FAILPOINT_DISABLE) -leak: gofail-enable +leak: failpoint-enable @export log_level=debug; \ - $(GOTEST) -tags leak $(PACKAGES) || { $(GOFAIL_DISABLE); exit 1; } - @$(GOFAIL_DISABLE) + $(GOTEST) -tags leak $(PACKAGES) || { $(FAILPOINT_DISABLE); exit 1; } + @$(FAILPOINT_DISABLE) -tikv_integration_test: gofail-enable - $(GOTEST) ./store/tikv/. -with-tikv=true || { $(GOFAIL_DISABLE); exit 1; } - @$(GOFAIL_DISABLE) +tikv_integration_test: failpoint-enable + $(GOTEST) ./store/tikv/. -with-tikv=true || { $(FAILPOINT_DISABLE); exit 1; } + @$(FAILPOINT_DISABLE) RACE_FLAG = ifeq ("$(WITH_RACE)", "1") @@ -181,13 +181,13 @@ importer: checklist: cat checklist.md -gofail-enable: tools/bin/gofail +failpoint-enable: tools/bin/failpoint-ctl # Converting gofail failpoints... - @$(GOFAIL_ENABLE) + @$(FAILPOINT_ENABLE) -gofail-disable: tools/bin/gofail +failpoint-disable: tools/bin/failpoint-ctl # Restoring gofail failpoints... - @$(GOFAIL_DISABLE) + @$(FAILPOINT_DISABLE) tools/bin/megacheck: tools/check/go.mod cd tools/check; \ @@ -213,5 +213,5 @@ tools/bin/errcheck: tools/check/go.mod cd tools/check; \ $(GO) build -o ../bin/errcheck github.com/kisielk/errcheck -tools/bin/gofail: go.mod - $(GO) build -o $@ github.com/pingcap/gofail +tools/bin/failpoint-ctl: go.mod + $(GO) build -o $@ github.com/pingcap/failpoint/failpoint-ctl diff --git a/ddl/column.go b/ddl/column.go index fd2a2587e7f17..8535c9d7adc9e 100644 --- a/ddl/column.go +++ b/ddl/column.go @@ -18,6 +18,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" @@ -137,10 +138,11 @@ func onAddColumn(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, err error) if err != nil { return ver, errors.Trace(err) } - // gofail: var errorBeforeDecodeArgs bool - // if errorBeforeDecodeArgs { - // return ver, errors.New("occur an error before decode args") - // } + failpoint.Inject("errorBeforeDecodeArgs", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(ver, errors.New("occur an error before decode args")) + } + }) col := &model.ColumnInfo{} pos := &ast.ColumnPosition{} offset := 0 @@ -336,12 +338,13 @@ func doModifyColumn(t *meta.Meta, job *model.Job, newCol *model.ColumnInfo, oldN } } - // gofail: var uninitializedOffsetAndState bool - // if uninitializedOffsetAndState { - // if newCol.State != model.StatePublic { - // return ver, errors.New("the column state is wrong") - // } - // } + failpoint.Inject("uninitializedOffsetAndState", func(val failpoint.Value) { + if val.(bool) { + if newCol.State != model.StatePublic { + failpoint.Return(ver, errors.New("the column state is wrong")) + } + } + }) // We need the latest column's offset and state. This information can be obtained from the store. newCol.Offset = oldCol.Offset diff --git a/ddl/fail_db_test.go b/ddl/fail_db_test.go index 0895f10906e00..988c4568384d8 100644 --- a/ddl/fail_db_test.go +++ b/ddl/fail_db_test.go @@ -21,7 +21,7 @@ import ( . "github.com/pingcap/check" "github.com/pingcap/errors" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser" "github.com/pingcap/parser/model" "github.com/pingcap/tidb/ddl" @@ -71,9 +71,10 @@ func (s *testFailDBSuite) TearDownSuite(c *C) { // TestHalfwayCancelOperations tests the case that the schema is correct after the execution of operations are cancelled halfway. func (s *testFailDBSuite) TestHalfwayCancelOperations(c *C) { - gofail.Enable("github.com/pingcap/tidb/ddl/truncateTableErr", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/ddl/truncateTableErr") - + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/truncateTableErr", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/truncateTableErr"), IsNil) + }() // test for truncating table _, err := s.se.Execute(context.Background(), "create database cancel_job_db") c.Assert(err, IsNil) @@ -110,8 +111,11 @@ func (s *testFailDBSuite) TestHalfwayCancelOperations(c *C) { c.Assert(err, IsNil) // test for renaming table - gofail.Enable("github.com/pingcap/tidb/ddl/renameTableErr", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/ddl/renameTableErr") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/renameTableErr", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/renameTableErr"), IsNil) + }() + _, err = s.se.Execute(context.Background(), "create table tx(a int)") c.Assert(err, IsNil) _, err = s.se.Execute(context.Background(), "insert into tx values(1)") @@ -155,15 +159,17 @@ func (s *testStateChangeSuite) TestInitializeOffsetAndState(c *C) { c.Assert(err, IsNil) defer s.se.Execute(context.Background(), "drop table t") - gofail.Enable("github.com/pingcap/tidb/ddl/uninitializedOffsetAndState", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/uninitializedOffsetAndState", `return(true)`), IsNil) _, err = s.se.Execute(context.Background(), "ALTER TABLE t MODIFY COLUMN b int FIRST;") c.Assert(err, IsNil) - gofail.Disable("github.com/pingcap/tidb/ddl/uninitializedOffsetAndState") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/uninitializedOffsetAndState"), IsNil) } func (s *testDBSuite) TestUpdateHandleFailed(c *C) { - gofail.Enable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle", `1*return`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/errorUpdateReorgHandle"), IsNil) + }() tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_handle_failed") defer tk.MustExec("drop database test_handle_failed") @@ -177,8 +183,10 @@ func (s *testDBSuite) TestUpdateHandleFailed(c *C) { } func (s *testDBSuite) TestAddIndexFailed(c *C) { - gofail.Enable("github.com/pingcap/tidb/ddl/mockAddIndexErr", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/ddl/mockAddIndexErr") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/mockAddIndexErr", `1*return`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/mockAddIndexErr"), IsNil) + }() tk := testkit.NewTestKit(c, s.store) tk.MustExec("create database if not exists test_add_index_failed") defer tk.MustExec("drop database test_add_index_failed") @@ -278,8 +286,10 @@ func (s *testDBSuite) TestAddIndexWorkerNum(c *C) { ddl.TestCheckWorkerNumber = lastSetWorkerCnt defer tk.MustExec(fmt.Sprintf("set @@global.tidb_ddl_reorg_worker_cnt=%d", originDDLAddIndexWorkerCnt)) - gofail.Enable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/checkIndexWorkerNum"), IsNil) + }() sessionExecInGoroutine(c, s.store, "create index c3_index on test_add_index (c3)", done) checkNum := 0 diff --git a/ddl/fail_test.go b/ddl/fail_test.go index 09507674ca92d..3349bcd38e893 100644 --- a/ddl/fail_test.go +++ b/ddl/fail_test.go @@ -15,7 +15,7 @@ package ddl import ( . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/tidb/types" @@ -51,10 +51,10 @@ func (s *testColumnChangeSuite) TestFailBeforeDecodeArgs(c *C) { stateCnt++ } else if job.SchemaState == model.StateWriteReorganization { if first { - gofail.Enable("github.com/pingcap/tidb/ddl/errorBeforeDecodeArgs", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/ddl/errorBeforeDecodeArgs", `return(true)`), IsNil) first = false } else { - gofail.Disable("github.com/pingcap/tidb/ddl/errorBeforeDecodeArgs") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/ddl/errorBeforeDecodeArgs"), IsNil) } } } diff --git a/ddl/index.go b/ddl/index.go index 4312d40dd8748..2b66ec2582222 100644 --- a/ddl/index.go +++ b/ddl/index.go @@ -21,6 +21,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/ast" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" @@ -837,8 +838,6 @@ func (w *addIndexWorker) handleBackfillTask(d *ddlCtx, task *reorgIndexTask) *ad return result } -var gofailMockAddindexErrOnceGuard bool - func (w *addIndexWorker) run(d *ddlCtx) { logutil.Logger(ddlLogCtx).Info("[ddl] add index worker start", zap.Int("workerID", w.id)) defer func() { @@ -857,13 +856,13 @@ func (w *addIndexWorker) run(d *ddlCtx) { } logutil.Logger(ddlLogCtx).Debug("[ddl] add index worker got task", zap.Int("workerID", w.id), zap.String("task", task.String())) - // gofail: var mockAddIndexErr bool - //if w.id == 0 && mockAddIndexErr && !gofailMockAddindexErrOnceGuard { - // gofailMockAddindexErrOnceGuard = true - // result := &addIndexResult{addedCount: 0, nextHandle: 0, err: errors.Errorf("mock add index error")} - // w.resultCh <- result - // continue - //} + failpoint.Inject("mockAddIndexErr", func() { + if w.id == 0 { + result := &addIndexResult{addedCount: 0, nextHandle: 0, err: errors.Errorf("mock add index error")} + w.resultCh <- result + failpoint.Continue() + } + }) // Dynamic change batch size. w.batchCnt = int(variable.GetDDLReorgBatchSize()) @@ -1137,20 +1136,21 @@ func (w *worker) addPhysicalTableIndex(t table.PhysicalTable, indexInfo *model.I closeAddIndexWorkers(workers) } - // gofail: var checkIndexWorkerNum bool - // if checkIndexWorkerNum { - // num := int(atomic.LoadInt32(&TestCheckWorkerNumber)) - // if num != 0 { - // if num > len(kvRanges) { - // if len(idxWorkers) != len(kvRanges) { - // return errors.Errorf("check index worker num error, len kv ranges is: %v, check index worker num is: %v, actual index num is: %v", len(kvRanges), num, len(idxWorkers)) - // } - // } else if num != len(idxWorkers) { - // return errors.Errorf("check index worker num error, len kv ranges is: %v, check index worker num is: %v, actual index num is: %v", len(kvRanges), num, len(idxWorkers)) - // } - // TestCheckWorkerNumCh <- struct{}{} - // } - //} + failpoint.Inject("checkIndexWorkerNum", func(val failpoint.Value) { + if val.(bool) { + num := int(atomic.LoadInt32(&TestCheckWorkerNumber)) + if num != 0 { + if num > len(kvRanges) { + if len(idxWorkers) != len(kvRanges) { + failpoint.Return(errors.Errorf("check index worker num error, len kv ranges is: %v, check index worker num is: %v, actual index num is: %v", len(kvRanges), num, len(idxWorkers))) + } + } else if num != len(idxWorkers) { + failpoint.Return(errors.Errorf("check index worker num error, len kv ranges is: %v, check index worker num is: %v, actual index num is: %v", len(kvRanges), num, len(idxWorkers))) + } + TestCheckWorkerNumCh <- struct{}{} + } + } + }) logutil.Logger(ddlLogCtx).Info("[ddl] start add index workers to reorg index", zap.Int("workerCnt", len(idxWorkers)), zap.Int("regionCnt", len(kvRanges)), zap.Int64("startHandle", startHandle), zap.Int64("endHandle", endHandle)) remains, err := w.sendRangeTaskToWorkers(t, idxWorkers, reorgInfo, &totalAddedCount, kvRanges) diff --git a/ddl/reorg.go b/ddl/reorg.go index 44a77354be2af..66c04a647b16d 100644 --- a/ddl/reorg.go +++ b/ddl/reorg.go @@ -21,6 +21,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/parser/terror" @@ -313,8 +314,6 @@ func (d *ddlCtx) GetTableMaxRowID(startTS uint64, tbl table.PhysicalTable) (maxR return maxRowID, false, nil } -var gofailOnceGuard bool - // getTableRange gets the start and end handle of a table (or partition). func getTableRange(d *ddlCtx, tbl table.PhysicalTable, snapshotVer uint64, priority int) (startHandle, endHandle int64, err error) { startHandle = math.MinInt64 @@ -376,12 +375,9 @@ func getReorgInfo(d *ddlCtx, t *meta.Meta, job *model.Job, tbl table.Table) (*re } logutil.Logger(ddlLogCtx).Info("[ddl] job get table range", zap.Int64("jobID", job.ID), zap.Int64("physicalTableID", pid), zap.Int64("startHandle", start), zap.Int64("endHandle", end)) - // gofail: var errorUpdateReorgHandle bool - // if errorUpdateReorgHandle && !gofailOnceGuard { - // // only return error once. - // gofailOnceGuard = true - // return &info, errors.New("occur an error when update reorg handle.") - // } + failpoint.Inject("errorUpdateReorgHandle", func() (*reorgInfo, error) { + return &info, errors.New("occur an error when update reorg handle") + }) err = t.UpdateDDLReorgHandle(job, start, end, pid) if err != nil { return &info, errors.Trace(err) diff --git a/ddl/table.go b/ddl/table.go index 60cc24a51513c..18378a014bcb6 100644 --- a/ddl/table.go +++ b/ddl/table.go @@ -19,6 +19,7 @@ import ( "strings" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" "github.com/pingcap/tidb/ddl/util" "github.com/pingcap/tidb/infoschema" @@ -257,11 +258,12 @@ func onTruncateTable(d *ddlCtx, t *meta.Meta, job *model.Job) (ver int64, _ erro job.State = model.JobStateCancelled return ver, errors.Trace(err) } - // gofail: var truncateTableErr bool - // if truncateTableErr { - // job.State = model.JobStateCancelled - // return ver, errors.New("occur an error after dropping table.") - // } + failpoint.Inject("truncateTableErr", func(val failpoint.Value) { + if val.(bool) { + job.State = model.JobStateCancelled + failpoint.Return(ver, errors.New("occur an error after dropping table")) + } + }) var oldPartitionIDs []int64 if tblInfo.GetPartitionInfo() != nil { @@ -388,11 +390,14 @@ func onRenameTable(t *meta.Meta, job *model.Job) (ver int64, _ error) { job.State = model.JobStateCancelled return ver, errors.Trace(err) } - // gofail: var renameTableErr bool - // if renameTableErr { - // job.State = model.JobStateCancelled - // return ver, errors.New("occur an error after renaming table.") - // } + + failpoint.Inject("renameTableErr", func(val failpoint.Value) { + if val.(bool) { + job.State = model.JobStateCancelled + failpoint.Return(ver, errors.New("occur an error after renaming table")) + } + }) + tblInfo.Name = tableName err = t.CreateTable(newSchemaID, tblInfo) if err != nil { diff --git a/executor/aggregate.go b/executor/aggregate.go index 55dea132a2c18..d80a83488ce9b 100644 --- a/executor/aggregate.go +++ b/executor/aggregate.go @@ -673,6 +673,7 @@ func (e *HashAggExec) execute(ctx context.Context) (err error) { if err != nil { return errors.Trace(err) } + // no more data. if e.childResult.NumRows() == 0 { return nil diff --git a/executor/executor_test.go b/executor/executor_test.go index 9cb29ada42929..4306ec8153210 100644 --- a/executor/executor_test.go +++ b/executor/executor_test.go @@ -28,7 +28,7 @@ import ( "github.com/golang/protobuf/proto" . "github.com/pingcap/check" "github.com/pingcap/errors" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" pb "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/log" "github.com/pingcap/parser" @@ -64,7 +64,6 @@ import ( "github.com/pingcap/tidb/util/testutil" "github.com/pingcap/tidb/util/timeutil" "github.com/pingcap/tipb/go-tipb" - "github.com/tiancaiamao/debugger" "go.uber.org/zap" "go.uber.org/zap/zapcore" "golang.org/x/net/context" @@ -1872,11 +1871,6 @@ func (s *testSuite) TestIsPointGet(c *C) { } func (s *testSuite) TestPointGetRepeatableRead(c *C) { - gofail.Enable("github.com/pingcap/tidb/executor/pointGetRepeatableReadTest1", `return(true)`) - gofail.Enable("github.com/pingcap/tidb/executor/pointGetRepeatableReadTest2", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/executor/pointGetRepeatableReadTest1") - defer gofail.Disable("github.com/pingcap/tidb/executor/pointGetRepeatableReadTest2") - tk1 := testkit.NewTestKit(c, s.store) tk1.MustExec("use test") tk1.MustExec(`create table point_get (a int, b int, c int, @@ -1886,19 +1880,30 @@ func (s *testSuite) TestPointGetRepeatableRead(c *C) { tk2 := testkit.NewTestKit(c, s.store) tk2.MustExec("use test") + var ( + step1 = "github.com/pingcap/tidb/executor/pointGetRepeatableReadTest-step1" + step2 = "github.com/pingcap/tidb/executor/pointGetRepeatableReadTest-step2" + ) + + c.Assert(failpoint.Enable(step1, "return"), IsNil) + c.Assert(failpoint.Enable(step2, "pause"), IsNil) + + updateWaitCh := make(chan struct{}) go func() { - ctx := context.WithValue(context.Background(), "pointGetRepeatableReadTest", true) + ctx := context.WithValue(context.Background(), "pointGetRepeatableReadTest", updateWaitCh) + ctx = failpoint.WithHook(ctx, func(ctx context.Context, fpname string) bool { + return fpname == step1 || fpname == step2 + }) rs, err := tk1.Se.Execute(ctx, "select c from point_get where b = 1") c.Assert(err, IsNil) result := tk1.ResultSetToResultWithCtx(ctx, rs[0], Commentf("execute sql fail")) result.Check(testkit.Rows("1")) }() - label := debugger.Bind("point-get-g2") - debugger.Continue("point-get-g1") - debugger.Breakpoint(label) + <-updateWaitCh // Wait `POINT GET` first time `get` + c.Assert(failpoint.Disable(step1), IsNil) tk2.MustExec("update point_get set b = 2, c = 2 where a = 1") - debugger.Continue("point-get-g1") + c.Assert(failpoint.Disable(step2), IsNil) } func (s *testSuite) TestRow(c *C) { @@ -2919,8 +2924,8 @@ func (s *testSuite) TestEarlyClose(c *C) { } // Goroutine should not leak when error happen. - gofail.Enable("github.com/pingcap/tidb/store/tikv/handleTaskOnceError", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/store/tikv/handleTaskOnceError") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/tikv/handleTaskOnceError", `return(true)`), IsNil) + defer func() { c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/handleTaskOnceError"), IsNil) }() rss, err := tk.Se.Execute(ctx, "select * from earlyclose") c.Assert(err, IsNil) rs := rss[0] diff --git a/executor/point_get.go b/executor/point_get.go index 1f906341ebf2b..1523f8ea90eb0 100644 --- a/executor/point_get.go +++ b/executor/point_get.go @@ -15,6 +15,7 @@ package executor import ( "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/expression" @@ -27,7 +28,6 @@ import ( "github.com/pingcap/tidb/types" "github.com/pingcap/tidb/util/chunk" "github.com/pingcap/tidb/util/codec" - "github.com/tiancaiamao/debugger" "golang.org/x/net/context" ) @@ -72,13 +72,6 @@ func (e *PointGetExecutor) Close() error { return nil } -func dummyUseDebuggerPackageToMakeGoLintHappy() { - // debugger is import and used in gofail only - // We need to **use** it, otherwise 'make check' would complain: - // imported and not used: "github.com/tiancaiamao/debugger" - debugger.Bind("xx") -} - // Next implements the Executor interface. func (e *PointGetExecutor) Next(ctx context.Context, chk *chunk.Chunk) error { chk.Reset() @@ -97,12 +90,6 @@ func (e *PointGetExecutor) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Trace(err1) } - // gofail: var pointGetRepeatableReadTest1 bool - // if pointGetRepeatableReadTest1 && ctx.Value("pointGetRepeatableReadTest") != nil { - // label := debugger.Bind("point-get-g1") - // debugger.Breakpoint(label) - // } - handleVal, err1 := e.get(idxKey) if err1 != nil && !kv.ErrNotExist.Equal(err1) { return errors.Trace(err1) @@ -115,12 +102,19 @@ func (e *PointGetExecutor) Next(ctx context.Context, chk *chunk.Chunk) error { return errors.Trace(err1) } - // gofail: var pointGetRepeatableReadTest2 bool - // if pointGetRepeatableReadTest2 && ctx.Value("pointGetRepeatableReadTest") != nil { - // label := debugger.Bind("point-get-g1") - // debugger.Continue("point-get-g2") - // debugger.Breakpoint(label) - // } + // The injection is used to simulate following scenario: + // 1. Session A create a point get query but pause before second time `GET` kv from backend + // 2. Session B create an UPDATE query to update the record that will be obtained in step 1 + // 3. Then point get retrieve data from backend after step 2 finished + // 4. Check the result + failpoint.InjectContext(ctx, "pointGetRepeatableReadTest-step1", func() { + if ch, ok := ctx.Value("pointGetRepeatableReadTest").(chan struct{}); ok { + // Make `UPDATE` continue + close(ch) + } + // Wait `UPDATE` finished + failpoint.InjectContext(ctx, "pointGetRepeatableReadTest-step2", nil) + }) } key := tablecodec.EncodeRowKeyWithHandle(e.tblInfo.ID, e.handle) diff --git a/executor/write_test.go b/executor/write_test.go index 79834bfceaf76..c1edbc38e0c11 100644 --- a/executor/write_test.go +++ b/executor/write_test.go @@ -19,7 +19,7 @@ import ( "sync/atomic" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/model" "github.com/pingcap/tidb/config" "github.com/pingcap/tidb/executor" @@ -2199,9 +2199,9 @@ func (s *testSuite) TestAutoIDInRetry(c *C) { tk.MustExec("insert into t values (),()") tk.MustExec("insert into t values ()") - gofail.Enable("github.com/pingcap/tidb/session/mockCommitRetryForAutoID", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/session/mockCommitRetryForAutoID", `return(true)`), IsNil) tk.MustExec("commit") - gofail.Disable("github.com/pingcap/tidb/session/mockCommitRetryForAutoID") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/session/mockCommitRetryForAutoID"), IsNil) tk.MustExec("insert into t values ()") tk.MustQuery(`select * from t`).Check(testkit.Rows("1", "2", "3", "4", "5")) diff --git a/go.mod b/go.mod index 15f312b33c6ba..d84815e5baf11 100644 --- a/go.mod +++ b/go.mod @@ -14,11 +14,9 @@ require ( github.com/coreos/pkg v0.0.0-20160727233714-3ac0863d7acf // indirect github.com/cznic/mathutil v0.0.0-20160613104831-78ad7f262603 github.com/cznic/sortutil v0.0.0-20150617083342-4c7342852e65 - github.com/davecgh/go-spew v1.1.1 // indirect github.com/dgrijalva/jwt-go v3.2.0+incompatible // indirect github.com/dustin/go-humanize v0.0.0-20180421182945-02af3965c54e // indirect github.com/eknkc/amber v0.0.0-20171010120322-cdade1c07385 // indirect - github.com/etcd-io/gofail v0.0.0-20180808172546-51ce9a71510a // indirect github.com/fsnotify/fsnotify v1.4.7 // indirect github.com/ghodss/yaml v1.0.0 // indirect github.com/go-sql-driver/mysql v0.0.0-20170715192408-3955978caca4 @@ -45,7 +43,7 @@ require ( github.com/opentracing/opentracing-go v1.0.2 github.com/pingcap/check v0.0.0-20190102082844-67f458068fc8 github.com/pingcap/errors v0.11.1 - github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3 + github.com/pingcap/failpoint v0.0.0-20190430075617-bf45ab20bfc4 github.com/pingcap/goleveldb v0.0.0-20171020084629-8d44bfdf1030 github.com/pingcap/kvproto v0.0.0-20190226063853-f6c0b7ffff11 github.com/pingcap/log v0.0.0-20190307075452-bd41d9273596 @@ -59,7 +57,6 @@ require ( github.com/prometheus/procfs v0.0.0-20180408092902-8b1c2da0d56d // indirect github.com/sirupsen/logrus v0.0.0-20170323161349-3bcb09397d6d github.com/spaolacci/murmur3 v0.0.0-20150829172844-0d12bf811670 - github.com/tiancaiamao/debugger v0.0.0-20190428065433-3a10ffa41d22 github.com/twinj/uuid v0.0.0-20150629100731-70cac2bcd273 github.com/uber/jaeger-client-go v2.8.0+incompatible github.com/uber/jaeger-lib v1.1.0 // indirect diff --git a/go.sum b/go.sum index baf5eb047b03d..adbf22768546e 100644 --- a/go.sum +++ b/go.sum @@ -33,8 +33,6 @@ github.com/dustin/go-humanize v0.0.0-20180421182945-02af3965c54e h1:Fw7ZmgiklsLh github.com/dustin/go-humanize v0.0.0-20180421182945-02af3965c54e/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/eknkc/amber v0.0.0-20171010120322-cdade1c07385 h1:clC1lXBpe2kTj2VHdaIu9ajZQe4kcEY9j0NsnDDBZ3o= github.com/eknkc/amber v0.0.0-20171010120322-cdade1c07385/go.mod h1:0vRUJqYpeSZifjYj7uP3BG/gKcuzL9xWVV/Y+cK33KM= -github.com/etcd-io/gofail v0.0.0-20180808172546-51ce9a71510a h1:QNEenQIsGDEEfFNSnN+h6hE1OwnHqTg7Dl9gEk1Cko4= -github.com/etcd-io/gofail v0.0.0-20180808172546-51ce9a71510a/go.mod h1:49H/RkXP8pKaZy4h0d+NW16rSLhyVBt4o6VLJbmOqDE= github.com/fsnotify/fsnotify v1.4.7 h1:IXs+QLmnXW2CcXuY+8Mzv/fWEsPGWxqefPtCP5CnV9I= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/ghodss/yaml v1.0.0 h1:wQHKEahhL6wmXdzwWG11gIVCkOv05bNOh+Rxn0yngAk= @@ -88,8 +86,7 @@ github.com/pingcap/check v0.0.0-20190102082844-67f458068fc8/go.mod h1:B1+S9LNcuM github.com/pingcap/errors v0.11.0/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= github.com/pingcap/errors v0.11.1 h1:BXFZ6MdDd2U1uJUa2sRAWTmm+nieEzuyYM0R4aUTcC8= github.com/pingcap/errors v0.11.1/go.mod h1:Oi8TUi2kEtXXLMJk9l1cGmz20kV3TaQ0usTwv5KuLY8= -github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3 h1:04yuCf5NMvLU8rB2m4Qs3rynH7EYpMno3lHkewIOdMo= -github.com/pingcap/gofail v0.0.0-20181217135706-6a951c1e42c3/go.mod h1:DazNTg0PTldtpsQiT9I5tVJwV1onHMKBBgXzmJUlMns= +github.com/pingcap/failpoint v0.0.0-20190430075617-bf45ab20bfc4/go.mod h1:p2F6D0adua5g+596cw96U8hU8slkeJhboEV7ySGDeEg= github.com/pingcap/goleveldb v0.0.0-20171020084629-8d44bfdf1030 h1:XJLuW0lsP7vAtQ2iPjZwvXZ14m5urp9No+Qr06ZZcTo= github.com/pingcap/goleveldb v0.0.0-20171020084629-8d44bfdf1030/go.mod h1:O17XtbryoCJhkKGbT62+L2OlrniwqiGLSqrmdHCMzZw= github.com/pingcap/kvproto v0.0.0-20190226063853-f6c0b7ffff11 h1:iGNfAHgK0VHJobW4bPTlFmdnt3YWsEHdSTIcjut6ffk= @@ -123,8 +120,6 @@ github.com/spaolacci/murmur3 v0.0.0-20150829172844-0d12bf811670/go.mod h1:JwIasO github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q= github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= -github.com/tiancaiamao/debugger v0.0.0-20190428065433-3a10ffa41d22 h1:P4sgavMKEdqNOws2VfR2c/Bye9nYFgV8gHyiW1wpQhE= -github.com/tiancaiamao/debugger v0.0.0-20190428065433-3a10ffa41d22/go.mod h1:qaShs3uDBYnvaQZJAJ6PjPg8kuAHR9zUJ8ilSLK1y/w= github.com/twinj/uuid v0.0.0-20150629100731-70cac2bcd273 h1:YqFyfcgqxQqjpRr0SEG0Z555J/3kPqDL/xmRyeAaX/0= github.com/twinj/uuid v0.0.0-20150629100731-70cac2bcd273/go.mod h1:mMgcE1RHFUFqe5AfiwlINXisXfDGro23fWdPUfOMjRY= github.com/uber/jaeger-client-go v2.8.0+incompatible h1:7DGH8Hqk6PirD+GE+bvCf0cLnspLuae7N1NcwMeQcyg= diff --git a/owner/fail_test.go b/owner/fail_test.go index a536e6fca7f80..dedd6fd33aca4 100644 --- a/owner/fail_test.go +++ b/owner/fail_test.go @@ -22,7 +22,7 @@ import ( "github.com/coreos/etcd/clientv3" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/util/logutil" "github.com/pingcap/tidb/util/testleak" @@ -75,7 +75,7 @@ func (s *testSuite) TestFailNewSession(c *C) { cli.Close() } }() - gofail.Enable("github.com/pingcap/tidb/owner/closeClient", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/owner/closeClient", `return(true)`), IsNil) _, err = NewSession(context.Background(), "fail_new_serssion", cli, retryCnt, ManagerSessionTTL) isContextDone := terror.ErrorEqual(grpc.ErrClientConnClosing, err) || terror.ErrorEqual(context.Canceled, err) c.Assert(isContextDone, IsTrue, Commentf("err %v", err)) @@ -91,7 +91,7 @@ func (s *testSuite) TestFailNewSession(c *C) { cli.Close() } }() - gofail.Enable("github.com/pingcap/tidb/owner/closeGrpc", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/owner/closeGrpc", `return(true)`), IsNil) _, err = NewSession(context.Background(), "fail_new_serssion", cli, retryCnt, ManagerSessionTTL) isContextDone := terror.ErrorEqual(grpc.ErrClientConnClosing, err) || terror.ErrorEqual(context.Canceled, err) c.Assert(isContextDone, IsTrue, Commentf("err %v", err)) diff --git a/owner/manager.go b/owner/manager.go index 3f717c9c1ee9e..a30cb2f1d09a5 100644 --- a/owner/manager.go +++ b/owner/manager.go @@ -27,6 +27,7 @@ import ( "github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes" "github.com/coreos/etcd/mvcc/mvccpb" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/metrics" "github.com/pingcap/tidb/util" @@ -142,15 +143,22 @@ func NewSession(ctx context.Context, logPrefix string, etcdCli *clientv3.Client, return etcdSession, errors.Trace(err) } - // gofail: var closeClient bool - // if closeClient { - // etcdCli.Close() - // } + failpoint.Inject("closeClient", func(val failpoint.Value) { + if val.(bool) { + if err := etcdCli.Close(); err != nil { + failpoint.Return(etcdSession, errors.Trace(err)) + } + } + }) + + failpoint.Inject("closeGrpc", func(val failpoint.Value) { + if val.(bool) { + if err := etcdCli.ActiveConnection().Close(); err != nil { + failpoint.Return(etcdSession, errors.Trace(err)) + } + } + }) - // gofail: var closeGrpc bool - // if closeGrpc { - // etcdCli.ActiveConnection().Close() - // } startTime := time.Now() etcdSession, err = concurrency.NewSession(etcdCli, concurrency.WithTTL(ttl), concurrency.WithContext(ctx)) diff --git a/session/session_fail_test.go b/session/session_fail_test.go index bf9bc61864040..a6ede00dc6a63 100644 --- a/session/session_fail_test.go +++ b/session/session_fail_test.go @@ -15,7 +15,7 @@ package session_test import ( . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/util/testkit" ) @@ -26,11 +26,11 @@ func (s *testSessionSuite) TestFailStatementCommit(c *C) { tk.MustExec("begin") tk.MustExec("insert into t values (1)") - gofail.Enable("github.com/pingcap/tidb/session/mockStmtCommitError", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/session/mockStmtCommitError", `return(true)`), IsNil) _, err := tk.Exec("insert into t values (2),(3),(4),(5)") c.Assert(err, NotNil) - gofail.Disable("github.com/pingcap/tidb/session/mockStmtCommitError") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/session/mockStmtCommitError"), IsNil) _, err = tk.Exec("select * from t") c.Assert(err, NotNil) @@ -65,12 +65,12 @@ func (s *testSessionSuite) TestFailStatementCommitInRetry(c *C) { tk.MustExec("insert into t values (2),(3),(4),(5)") tk.MustExec("insert into t values (6)") - gofail.Enable("github.com/pingcap/tidb/session/mockCommitError8942", `return(true)`) - gofail.Enable("github.com/pingcap/tidb/session/mockStmtCommitError", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/session/mockCommitError8942", `return(true)`), IsNil) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/session/mockStmtCommitError", `return(true)`), IsNil) _, err := tk.Exec("commit") c.Assert(err, NotNil) - gofail.Disable("github.com/pingcap/tidb/session/mockCommitError8942") - gofail.Disable("github.com/pingcap/tidb/session/mockStmtCommitError") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/session/mockCommitError8942"), IsNil) + c.Assert(failpoint.Disable("github.com/pingcap/tidb/session/mockStmtCommitError"), IsNil) tk.MustExec("insert into t values (6)") tk.MustQuery(`select * from t`).Check(testkit.Rows("6")) diff --git a/session/txn.go b/session/txn.go index 24df05c0b794f..8d5d0ede24609 100644 --- a/session/txn.go +++ b/session/txn.go @@ -19,6 +19,7 @@ import ( "sync/atomic" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/executor" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx" @@ -176,17 +177,19 @@ func (st *TxnState) Commit(ctx context.Context) error { } // mockCommitError8942 is used for PR #8942. - // gofail: var mockCommitError8942 bool - // if mockCommitError8942 { - // return kv.ErrRetryable - // } + failpoint.Inject("mockCommitError8942", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(kv.ErrRetryable) + } + }) // mockCommitRetryForAutoID is used to mock an commit retry for adjustAutoIncrementDatum. - // gofail: var mockCommitRetryForAutoID bool - // if mockCommitRetryForAutoID && !mockAutoIDRetry() { - // enableMockAutoIDRetry() - // return kv.ErrRetryable - // } + failpoint.Inject("mockCommitRetryForAutoID", func(val failpoint.Value) { + if val.(bool) && !mockAutoIDRetry() { + enableMockAutoIDRetry() + failpoint.Return(kv.ErrRetryable) + } + }) return errors.Trace(st.Transaction.Commit(ctx)) } @@ -361,11 +364,12 @@ func (s *session) StmtCommit() error { st := &s.txn var count int err := kv.WalkMemBuffer(st.buf, func(k kv.Key, v []byte) error { + failpoint.Inject("mockStmtCommitError", func(val failpoint.Value) { + if val.(bool) { + count++ + } + }) - // gofail: var mockStmtCommitError bool - // if mockStmtCommitError { - // count++ - // } if count > 3 { return errors.New("mock stmt commit error") } diff --git a/store/mockstore/mocktikv/rpc.go b/store/mockstore/mocktikv/rpc.go index 08cdcbe9d6737..1303281cb9d8d 100644 --- a/store/mockstore/mocktikv/rpc.go +++ b/store/mockstore/mocktikv/rpc.go @@ -21,6 +21,7 @@ import ( "github.com/golang/protobuf/proto" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/coprocessor" "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/kvproto/pkg/kvrpcpb" @@ -547,10 +548,12 @@ func (c *RPCClient) checkArgs(ctx context.Context, addr string) (*rpcHandler, er // SendRequest sends a request to mock cluster. func (c *RPCClient) SendRequest(ctx context.Context, addr string, req *tikvrpc.Request, timeout time.Duration) (*tikvrpc.Response, error) { - // gofail: var rpcServerBusy bool - // if rpcServerBusy { - // return tikvrpc.GenRegionErrorResp(req, &errorpb.Error{ServerIsBusy: &errorpb.ServerIsBusy{}}) - // } + failpoint.Inject("rpcServerBusy", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(tikvrpc.GenRegionErrorResp(req, &errorpb.Error{ServerIsBusy: &errorpb.ServerIsBusy{}})) + } + }) + handler, err := c.checkArgs(ctx, addr) if err != nil { return nil, err @@ -582,31 +585,34 @@ func (c *RPCClient) SendRequest(ctx context.Context, addr string, req *tikvrpc.R } resp.Prewrite = handler.handleKvPrewrite(r) case tikvrpc.CmdCommit: - // gofail: var rpcCommitResult string - // switch rpcCommitResult { - // case "timeout": - // return nil, errors.New("timeout") - // case "notLeader": - // return &tikvrpc.Response{ - // Type: tikvrpc.CmdCommit, - // Commit: &kvrpcpb.CommitResponse{RegionError: &errorpb.Error{NotLeader: &errorpb.NotLeader{}}}, - // }, nil - // case "keyError": - // return &tikvrpc.Response{ - // Type: tikvrpc.CmdCommit, - // Commit: &kvrpcpb.CommitResponse{Error: &kvrpcpb.KeyError{}}, - // }, nil - // } + failpoint.Inject("rpcCommitResult", func(val failpoint.Value) { + switch val.(string) { + case "timeout": + failpoint.Return(nil, errors.New("timeout")) + case "notLeader": + failpoint.Return(&tikvrpc.Response{ + Type: tikvrpc.CmdCommit, + Commit: &kvrpcpb.CommitResponse{RegionError: &errorpb.Error{NotLeader: &errorpb.NotLeader{}}}, + }, nil) + case "keyError": + failpoint.Return(&tikvrpc.Response{ + Type: tikvrpc.CmdCommit, + Commit: &kvrpcpb.CommitResponse{Error: &kvrpcpb.KeyError{}}, + }, nil) + } + }) + r := req.Commit if err := handler.checkRequest(reqCtx, r.Size()); err != nil { resp.Commit = &kvrpcpb.CommitResponse{RegionError: err} return resp, nil } resp.Commit = handler.handleKvCommit(r) - // gofail: var rpcCommitTimeout bool - // if rpcCommitTimeout { - // return nil, undeterminedErr - // } + failpoint.Inject("rpcCommitTimeout", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(nil, undeterminedErr) + } + }) case tikvrpc.CmdCleanup: r := req.Cleanup if err := handler.checkRequest(reqCtx, r.Size()); err != nil { diff --git a/store/tikv/2pc_fail_test.go b/store/tikv/2pc_fail_test.go index 32b1b6831d577..e6fe006beff80 100644 --- a/store/tikv/2pc_fail_test.go +++ b/store/tikv/2pc_fail_test.go @@ -16,7 +16,7 @@ package tikv import ( . "github.com/pingcap/check" "github.com/pingcap/errors" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/kv" "golang.org/x/net/context" @@ -25,8 +25,10 @@ import ( // TestFailCommitPrimaryRpcErrors tests rpc errors are handled properly when // committing primary region task. func (s *testCommitterSuite) TestFailCommitPrimaryRpcErrors(c *C) { - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("timeout")`) - defer gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("timeout")`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult"), IsNil) + }() // The rpc error will be wrapped to ErrResultUndetermined. t1 := s.begin(c) err := t1.Set([]byte("a"), []byte("a1")) @@ -43,8 +45,10 @@ func (s *testCommitterSuite) TestFailCommitPrimaryRpcErrors(c *C) { // TestFailCommitPrimaryRegionError tests RegionError is handled properly when // committing primary region task. func (s *testCommitterSuite) TestFailCommitPrimaryRegionError(c *C) { - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("notLeader")`) - defer gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("notLeader")`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult"), IsNil) + }() // Ensure it returns the original error without wrapped to ErrResultUndetermined // if it exceeds max retry timeout on RegionError. t2 := s.begin(c) @@ -58,8 +62,10 @@ func (s *testCommitterSuite) TestFailCommitPrimaryRegionError(c *C) { // TestFailCommitPrimaryRPCErrorThenRegionError tests the case when commit first // receive a rpc timeout, then region errors afterwrards. func (s *testCommitterSuite) TestFailCommitPrimaryRPCErrorThenRegionError(c *C) { - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `1*return("timeout")->return("notLeader")`) - defer gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `1*return("timeout")->return("notLeader")`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult"), IsNil) + }() // The region error will be wrapped to ErrResultUndetermined. t1 := s.begin(c) err := t1.Set([]byte("a"), []byte("a1")) @@ -72,8 +78,10 @@ func (s *testCommitterSuite) TestFailCommitPrimaryRPCErrorThenRegionError(c *C) // TestFailCommitPrimaryKeyError tests KeyError is handled properly when // committing primary region task. func (s *testCommitterSuite) TestFailCommitPrimaryKeyError(c *C) { - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("keyError")`) - defer gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("keyError")`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult"), IsNil) + }() // Ensure it returns the original error without wrapped to ErrResultUndetermined // if it meets KeyError. t3 := s.begin(c) @@ -85,8 +93,10 @@ func (s *testCommitterSuite) TestFailCommitPrimaryKeyError(c *C) { } func (s *testCommitterSuite) TestFailCommitTimeout(c *C) { - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitTimeout", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitTimeout") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitTimeout", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitTimeout"), IsNil) + }() txn := s.begin(c) err := txn.Set([]byte("a"), []byte("a1")) c.Assert(err, IsNil) diff --git a/store/tikv/coprocessor.go b/store/tikv/coprocessor.go index c11b4718c2d0c..2e1a1b021bf03 100644 --- a/store/tikv/coprocessor.go +++ b/store/tikv/coprocessor.go @@ -26,6 +26,7 @@ import ( "github.com/cznic/mathutil" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/coprocessor" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/tidb/kv" @@ -625,11 +626,11 @@ func (worker *copIteratorWorker) handleTask(bo *Backoffer, task *copTask, respCh // handleTaskOnce handles single copTask, successful results are send to channel. // If error happened, returns error. If region split or meet lock, returns the remain tasks. func (worker *copIteratorWorker) handleTaskOnce(bo *Backoffer, task *copTask, ch chan<- *copResponse) ([]*copTask, error) { - - // gofail: var handleTaskOnceError bool - // if handleTaskOnceError { - // return nil, errors.New("mock handleTaskOnce error") - // } + failpoint.Inject("handleTaskOnceError", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(nil, errors.New("mock handleTaskOnce error")) + } + }) sender := NewRegionRequestSender(worker.store.regionCache, worker.store.client) req := &tikvrpc.Request{ diff --git a/store/tikv/gcworker/gc_worker_test.go b/store/tikv/gcworker/gc_worker_test.go index fa7a212b4de48..852888e8bc6d1 100644 --- a/store/tikv/gcworker/gc_worker_test.go +++ b/store/tikv/gcworker/gc_worker_test.go @@ -20,7 +20,7 @@ import ( "time" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/session" @@ -161,23 +161,23 @@ func (s *testGCWorkerSuite) TestDoGCForOneRegion(c *C) { c.Assert(regionErr, IsNil) c.Assert(err, IsNil) - gofail.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("timeout")`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("timeout")`), IsNil) regionErr, err = taskWorker.doGCForRegion(bo, 20, loc.Region) c.Assert(regionErr, IsNil) c.Assert(err, NotNil) - gofail.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult"), IsNil) - gofail.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("GCNotLeader")`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("GCNotLeader")`), IsNil) regionErr, err = taskWorker.doGCForRegion(bo, 20, loc.Region) c.Assert(regionErr.GetNotLeader(), NotNil) c.Assert(err, IsNil) - gofail.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult"), IsNil) - gofail.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("GCServerIsBusy")`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult", `return("GCServerIsBusy")`), IsNil) regionErr, err = taskWorker.doGCForRegion(bo, 20, loc.Region) c.Assert(regionErr.GetServerIsBusy(), NotNil) c.Assert(err, IsNil) - gofail.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/tikv/tikvStoreSendReqResult"), IsNil) } func (s *testGCWorkerSuite) TestDoGC(c *C) { diff --git a/store/tikv/region_request.go b/store/tikv/region_request.go index af16af5beaace..1e186a6e77f30 100644 --- a/store/tikv/region_request.go +++ b/store/tikv/region_request.go @@ -17,6 +17,7 @@ import ( "time" "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/kvproto/pkg/errorpb" "github.com/pingcap/kvproto/pkg/kvrpcpb" "github.com/pingcap/tidb/kv" @@ -61,26 +62,26 @@ func NewRegionRequestSender(regionCache *RegionCache, client Client) *RegionRequ // SendReq sends a request to tikv server. func (s *RegionRequestSender) SendReq(bo *Backoffer, req *tikvrpc.Request, regionID RegionVerID, timeout time.Duration) (*tikvrpc.Response, error) { - - // gofail: var tikvStoreSendReqResult string - // switch tikvStoreSendReqResult { - // case "timeout": - // return nil, errors.New("timeout") - // case "GCNotLeader": - // if req.Type == tikvrpc.CmdGC { - // return &tikvrpc.Response{ - // Type: tikvrpc.CmdGC, - // GC: &kvrpcpb.GCResponse{RegionError: &errorpb.Error{NotLeader: &errorpb.NotLeader{}}}, - // }, nil - // } - // case "GCServerIsBusy": - // if req.Type == tikvrpc.CmdGC { - // return &tikvrpc.Response{ - // Type: tikvrpc.CmdGC, - // GC: &kvrpcpb.GCResponse{RegionError: &errorpb.Error{ServerIsBusy: &errorpb.ServerIsBusy{}}}, - // }, nil - // } - // } + failpoint.Inject("tikvStoreSendReqResult", func(val failpoint.Value) { + switch val.(string) { + case "timeout": + failpoint.Return(nil, errors.New("timeout")) + case "GCNotLeader": + if req.Type == tikvrpc.CmdGC { + failpoint.Return(&tikvrpc.Response{ + Type: tikvrpc.CmdGC, + GC: &kvrpcpb.GCResponse{RegionError: &errorpb.Error{NotLeader: &errorpb.NotLeader{}}}, + }, nil) + } + case "GCServerIsBusy": + if req.Type == tikvrpc.CmdGC { + failpoint.Return(&tikvrpc.Response{ + Type: tikvrpc.CmdGC, + GC: &kvrpcpb.GCResponse{RegionError: &errorpb.Error{ServerIsBusy: &errorpb.ServerIsBusy{}}}, + }, nil) + } + } + }) for { ctx, err := s.regionCache.GetRPCContext(bo, regionID) diff --git a/store/tikv/sql_fail_test.go b/store/tikv/sql_fail_test.go index fbbfc70c8ade7..7cc640cbab641 100644 --- a/store/tikv/sql_fail_test.go +++ b/store/tikv/sql_fail_test.go @@ -19,7 +19,7 @@ import ( "time" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/terror" "github.com/pingcap/tidb/domain" "github.com/pingcap/tidb/session" @@ -58,11 +58,11 @@ func (s *testSQLSuite) TestFailBusyServerCop(c *C) { var wg sync.WaitGroup wg.Add(2) - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy", `return(true)`), IsNil) go func() { defer wg.Done() time.Sleep(time.Millisecond * 100) - gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy"), IsNil) }() go func() { diff --git a/store/tikv/store_fail_test.go b/store/tikv/store_fail_test.go index 01bc02ba61245..c008ce2642069 100644 --- a/store/tikv/store_fail_test.go +++ b/store/tikv/store_fail_test.go @@ -18,7 +18,7 @@ import ( "time" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "golang.org/x/net/context" ) @@ -33,11 +33,11 @@ func (s *testStoreSuite) TestFailBusyServerKV(c *C) { var wg sync.WaitGroup wg.Add(2) - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy", `return(true)`) + c.Assert(failpoint.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy", `return(true)`), IsNil) go func() { defer wg.Done() time.Sleep(time.Millisecond * 100) - gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy") + c.Assert(failpoint.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcServerBusy"), IsNil) }() go func() { diff --git a/tablecodec/tablecodec_test.go b/tablecodec/tablecodec_test.go index 60448b94376c5..a942a01744d4a 100644 --- a/tablecodec/tablecodec_test.go +++ b/tablecodec/tablecodec_test.go @@ -20,7 +20,7 @@ import ( "time" . "github.com/pingcap/check" - gofail "github.com/pingcap/gofail/runtime" + "github.com/pingcap/failpoint" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/kv" "github.com/pingcap/tidb/sessionctx/stmtctx" @@ -282,8 +282,10 @@ func (s *testTableCodecSuite) TestCutKey(c *C) { } func (s *testTableCodecSuite) TestDecodeBadDecical(c *C) { - gofail.Enable("github.com/pingcap/tidb/util/codec/errorInDecodeDecimal", `return(true)`) - defer gofail.Disable("github.com/pingcap/tidb/util/codec/errorInDecodeDecimal") + c.Assert(failpoint.Enable("github.com/pingcap/tidb/util/codec/errorInDecodeDecimal", `return(true)`), IsNil) + defer func() { + c.Assert(failpoint.Disable("github.com/pingcap/tidb/util/codec/errorInDecodeDecimal"), IsNil) + }() dec := types.NewDecFromStringForTest("0.111") b, err := codec.EncodeDecimal(nil, dec, 0, 0) c.Assert(err, IsNil) diff --git a/util/codec/decimal.go b/util/codec/decimal.go index 8fcb849c70d23..abf7130a7f4b9 100644 --- a/util/codec/decimal.go +++ b/util/codec/decimal.go @@ -15,6 +15,7 @@ package codec import ( "github.com/pingcap/errors" + "github.com/pingcap/failpoint" "github.com/pingcap/tidb/types" ) @@ -31,10 +32,11 @@ func EncodeDecimal(b []byte, dec *types.MyDecimal, precision, frac int) ([]byte, // DecodeDecimal decodes bytes to decimal. func DecodeDecimal(b []byte) ([]byte, *types.MyDecimal, int, int, error) { - // gofail: var errorInDecodeDecimal bool - // if errorInDecodeDecimal { - // return b, nil, 0, 0, errors.New("gofail error") - // } + failpoint.Inject("errorInDecodeDecimal", func(val failpoint.Value) { + if val.(bool) { + failpoint.Return(b, nil, 0, 0, errors.New("gofail error")) + } + }) if len(b) < 3 { return b, nil, 0, 0, errors.New("insufficient bytes to decode value")