diff --git a/Makefile b/Makefile index 3c5e43415..576379c27 100644 --- a/Makefile +++ b/Makefile @@ -182,6 +182,8 @@ static: prepare tools @# exhaustivestruct - Protobuf structs have hidden fields, like "XXX_NoUnkeyedLiteral" @# exhaustive - no need to check exhaustiveness of enum switch statements @# gosec - too many false positive + @# errorlint - pingcap/errors is incompatible with std errors. + @# wrapcheck - there are too many unwrapped errors in tidb-lightning CGO_ENABLED=0 tools/bin/golangci-lint run --enable-all --deadline 120s \ --disable gochecknoglobals \ --disable goimports \ @@ -202,7 +204,9 @@ static: prepare tools --disable exhaustive \ --disable godot \ --disable gosec \ - $$($(PACKAGE_DIRECTORIES) | grep -v "lightning") + --disable errorlint \ + --disable wrapcheck \ + $(PACKAGE_DIRECTORIES) # pingcap/errors APIs are mixed with multiple patterns 'pkg/errors', # 'juju/errors' and 'pingcap/parser'. To avoid confusion and mistake, # we only allow a subset of APIs, that's "Normalize|Annotate|Trace|Cause". diff --git a/cmd/tidb-lightning-ctl/main.go b/cmd/tidb-lightning-ctl/main.go index 47d8bc8ba..a7f6d0574 100644 --- a/cmd/tidb-lightning-ctl/main.go +++ b/cmd/tidb-lightning-ctl/main.go @@ -61,7 +61,7 @@ func run() error { // there is a check if `-d` points to a valid storage, and '' is not. // since tidb-lightning-ctl does not need `-d` we change the default to a valid but harmless value. dFlag := fs.Lookup("d") - dFlag.Value.Set("noop://") + _ = dFlag.Value.Set("noop://") dFlag.DefValue = "noop://" compact = fs.Bool("compact", false, "do manual compaction on the target cluster") @@ -247,8 +247,8 @@ func checkpointErrorDestroy(ctx context.Context, cfg *config.Config, tls *common if err != nil { fmt.Fprintln(os.Stderr, "* Encountered error while closing engine:", err) lastErr = err - } else { - closedEngine.Cleanup(ctx) + } else if err := closedEngine.Cleanup(ctx); err != nil { + lastErr = err } } } @@ -263,7 +263,7 @@ func checkpointErrorDestroy(ctx context.Context, cfg *config.Config, tls *common for engineID := table.MinEngineID; engineID <= table.MaxEngineID; engineID++ { fmt.Fprintln(os.Stderr, "Closing and cleaning up engine:", table.TableName, engineID) _, eID := backend.MakeUUID(table.TableName, engineID) - file := local.File{Uuid: eID} + file := local.File{UUID: eID} err := file.Cleanup(cfg.TikvImporter.SortedKVDir) if err != nil { fmt.Fprintln(os.Stderr, "* Encountered error while cleanup engine:", err) @@ -321,6 +321,7 @@ func checkpointDump(ctx context.Context, cfg *config.Config, dumpFolder string) } func getLocalStoringTables(ctx context.Context, cfg *config.Config) (err2 error) { + //nolint:prealloc // This is a placeholder. var tables []string defer func() { if err2 == nil { diff --git a/cmd/tidb-lightning-ctl/main_test.go b/cmd/tidb-lightning-ctl/main_test.go index bef107137..e0351c20e 100644 --- a/cmd/tidb-lightning-ctl/main_test.go +++ b/cmd/tidb-lightning-ctl/main_test.go @@ -19,7 +19,7 @@ import ( "testing" ) -func TestRunMain(t *testing.T) { +func TestRunMain(_ *testing.T) { if _, isIntegrationTest := os.LookupEnv("INTEGRATION_TEST"); !isIntegrationTest { // override exit to pass unit test. exit = func(code int) {} diff --git a/cmd/tidb-lightning/main.go b/cmd/tidb-lightning/main.go index 7bb40f734..c5f967ba0 100644 --- a/cmd/tidb-lightning/main.go +++ b/cmd/tidb-lightning/main.go @@ -78,13 +78,12 @@ func main() { err = func() error { if globalCfg.App.ServerMode { return app.RunServer() - } else { - cfg := config.NewConfig() - if err := cfg.LoadFromGlobal(globalCfg); err != nil { - return err - } - return app.RunOnce(context.Background(), cfg, nil) } + cfg := config.NewConfig() + if err := cfg.LoadFromGlobal(globalCfg); err != nil { + return err + } + return app.RunOnce(context.Background(), cfg, nil) }() if err != nil { diff --git a/cmd/tidb-lightning/main_test.go b/cmd/tidb-lightning/main_test.go index 26ee27d45..c77e38a88 100644 --- a/cmd/tidb-lightning/main_test.go +++ b/cmd/tidb-lightning/main_test.go @@ -21,7 +21,7 @@ import ( "testing" ) -func TestRunMain(t *testing.T) { +func TestRunMain(_ *testing.T) { if _, isIntegrationTest := os.LookupEnv("INTEGRATION_TEST"); !isIntegrationTest { // override exit to pass unit test. exit = func(code int) {} diff --git a/pkg/lightning/backend/backend_test.go b/pkg/lightning/backend/backend_test.go index 5094420ca..0a09a12a6 100644 --- a/pkg/lightning/backend/backend_test.go +++ b/pkg/lightning/backend/backend_test.go @@ -33,7 +33,7 @@ func Test(t *testing.T) { // FIXME: Cannot use the real SetUpTest/TearDownTest to set up the mock // otherwise the mock error will be ignored. -func (s *backendSuite) setUpTest(c *C) { +func (s *backendSuite) setUpTest(c gomock.TestReporter) { s.controller = gomock.NewController(c) s.mockBackend = mock.NewMockBackend(s.controller) s.backend = backend.MakeBackend(s.mockBackend) @@ -298,6 +298,7 @@ func (s *backendSuite) TestImportFailedRecovered(c *C) { c.Assert(err, IsNil) } +//nolint:interfacer // change test case signature causes check panicking. func (s *backendSuite) TestClose(c *C) { s.setUpTest(c) defer s.tearDownTest() diff --git a/pkg/lightning/backend/importer/importer.go b/pkg/lightning/backend/importer/importer.go index 9121d61d8..0222b3694 100644 --- a/pkg/lightning/backend/importer/importer.go +++ b/pkg/lightning/backend/importer/importer.go @@ -202,6 +202,7 @@ outside: func (importer *importer) WriteRowsToImporter( ctx context.Context, + //nolint:interfacer // false positive engineUUID uuid.UUID, ts uint64, rows kv.Rows, @@ -326,18 +327,18 @@ func (importer *importer) ResetEngine(context.Context, uuid.UUID) error { } func (importer *importer) LocalWriter(ctx context.Context, engineUUID uuid.UUID) (backend.EngineWriter, error) { - return &ImporterWriter{importer: importer, engineUUID: engineUUID}, nil + return &Writer{importer: importer, engineUUID: engineUUID}, nil } -type ImporterWriter struct { +type Writer struct { importer *importer engineUUID uuid.UUID } -func (w *ImporterWriter) Close() error { +func (w *Writer) Close() error { return nil } -func (w *ImporterWriter) AppendRows(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { +func (w *Writer) AppendRows(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { return w.importer.WriteRows(ctx, w.engineUUID, tableName, columnNames, ts, rows) } diff --git a/pkg/lightning/backend/kv/session.go b/pkg/lightning/backend/kv/session.go index 9e0573dc1..da8a812bc 100644 --- a/pkg/lightning/backend/kv/session.go +++ b/pkg/lightning/backend/kv/session.go @@ -27,6 +27,9 @@ import ( "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/br/pkg/lightning/common" + "github.com/pingcap/br/pkg/lightning/log" + + "go.uber.org/zap" ) // invalidIterator is a trimmed down Iterator type which is invalid. @@ -182,11 +185,18 @@ func newSession(options *SessionOptions) *session { vars.SQLMode = sqlMode if options.SysVars != nil { for k, v := range options.SysVars { - vars.SetSystemVar(k, v) + if err := vars.SetSystemVar(k, v); err != nil { + log.L().DPanic("new session: failed to set system var", + log.ShortError(err), + zap.String("key", k)) + } } } vars.StmtCtx.TimeZone = vars.Location() - vars.SetSystemVar("timestamp", strconv.FormatInt(options.Timestamp, 10)) + if err := vars.SetSystemVar("timestamp", strconv.FormatInt(options.Timestamp, 10)); err != nil { + log.L().Warn("new session: failed to set timestamp", + log.ShortError(err)) + } vars.TxnCtx = nil s := &session{ diff --git a/pkg/lightning/backend/kv/sql2kv.go b/pkg/lightning/backend/kv/sql2kv.go index 711a013b1..11c509724 100644 --- a/pkg/lightning/backend/kv/sql2kv.go +++ b/pkg/lightning/backend/kv/sql2kv.go @@ -93,7 +93,7 @@ func autoRandomIncrementBits(col *table.Column, randomBits int) int { incrementalBits := typeBitsLength - randomBits hasSignBit := !mysql.HasUnsignedFlag(col.Flag) if hasSignBit { - incrementalBits -= 1 + incrementalBits-- } return incrementalBits } @@ -211,11 +211,13 @@ func (row RowArrayMarshaler) MarshalLogArray(encoder zapcore.ArrayEncoder) error return err } } - encoder.AppendObject(zapcore.ObjectMarshalerFunc(func(enc zapcore.ObjectEncoder) error { + if err := encoder.AppendObject(zapcore.ObjectMarshalerFunc(func(enc zapcore.ObjectEncoder) error { enc.AddString("kind", kindStr[kind]) enc.AddString("val", log.RedactString(str)) return nil - })) + })); err != nil { + return err + } } return nil } @@ -277,8 +279,9 @@ func MakeRowFromKvPairs(pairs []common.KvPair) Row { // KvPairsFromRows converts a Rows instance constructed from MakeRowsFromKvPairs // back into a slice of KvPair. This method panics if the Rows is not // constructed in such way. +// nolint:golint // kv.KvPairsFromRows sounds good. func KvPairsFromRows(rows Rows) []common.KvPair { - return []common.KvPair(rows.(kvPairs)) + return rows.(kvPairs) } // Encode a row of data into KV pairs. @@ -295,6 +298,7 @@ func (kvcodec *tableKVEncoder) Encode( var value types.Datum var err error + //nolint:prealloc // This is a placeholder. var record []types.Datum if kvcodec.recordCache != nil { @@ -308,15 +312,16 @@ func (kvcodec *tableKVEncoder) Encode( j := columnPermutation[i] isAutoIncCol := mysql.HasAutoIncrementFlag(col.Flag) isPk := mysql.HasPriKeyFlag(col.Flag) - if j >= 0 && j < len(row) { + switch { + case j >= 0 && j < len(row): value, err = table.CastValue(kvcodec.se, row[j], col.ToInfo(), false, false) if err == nil { value, err = col.HandleBadNull(value, kvcodec.se.vars.StmtCtx) } - } else if isAutoIncCol { + case isAutoIncCol: // we still need a conversion, e.g. to catch overflow with a TINYINT column. value, err = table.CastValue(kvcodec.se, types.NewIntDatum(rowID), col.ToInfo(), false, false) - } else if isAutoRandom && isPk { + case isAutoRandom && isPk: var val types.Datum if mysql.HasUnsignedFlag(col.Flag) { val = types.NewUintDatum(uint64(kvcodec.autoRandomHeaderBits | rowID)) @@ -324,11 +329,11 @@ func (kvcodec *tableKVEncoder) Encode( val = types.NewIntDatum(kvcodec.autoRandomHeaderBits | rowID) } value, err = table.CastValue(kvcodec.se, val, col.ToInfo(), false, false) - } else if col.IsGenerated() { + case col.IsGenerated(): // inject some dummy value for gen col so that MutRowFromDatums below sees a real value instead of nil. // if MutRowFromDatums sees a nil it won't initialize the underlying storage and cause SetDatum to panic. value = types.GetMinValue(&col.FieldType) - } else { + default: value, err = table.GetColDefaultValue(kvcodec.se, col.ToInfo()) } if err != nil { @@ -339,10 +344,14 @@ func (kvcodec *tableKVEncoder) Encode( if isAutoRandom && isPk { incrementalBits := autoRandomIncrementBits(col, int(kvcodec.tbl.Meta().AutoRandomBits)) - kvcodec.tbl.RebaseAutoID(kvcodec.se, value.GetInt64()&((1< 0 { @@ -414,8 +425,8 @@ func (kvs kvPairs) ClassifyAndAppend( *indices = indexKVs } -func (totalKVs kvPairs) SplitIntoChunks(splitSize int) []Rows { - if len(totalKVs) == 0 { +func (kvs kvPairs) SplitIntoChunks(splitSize int) []Rows { + if len(kvs) == 0 { return nil } @@ -423,19 +434,19 @@ func (totalKVs kvPairs) SplitIntoChunks(splitSize int) []Rows { i := 0 cumSize := 0 - for j, pair := range totalKVs { + for j, pair := range kvs { size := len(pair.Key) + len(pair.Val) if i < j && cumSize+size > splitSize { - res = append(res, kvPairs(totalKVs[i:j])) + res = append(res, kvs[i:j]) i = j cumSize = 0 } cumSize += size } - return append(res, kvPairs(totalKVs[i:])) + return append(res, kvs[i:]) } func (kvs kvPairs) Clear() Rows { - return kvPairs(kvs[:0]) + return kvs[:0] } diff --git a/pkg/lightning/backend/kv/sql2kv_test.go b/pkg/lightning/backend/kv/sql2kv_test.go index 81baf57fa..2eba04be4 100644 --- a/pkg/lightning/backend/kv/sql2kv_test.go +++ b/pkg/lightning/backend/kv/sql2kv_test.go @@ -94,7 +94,7 @@ func (s *kvSuite) TestEncode(c *C) { types.NewIntDatum(1), types.NewStringDatum("invalid-pk"), } - pairs, err = strictMode.Encode(logger, rowsWithPk, 2, []int{0, 1}) + _, err = strictMode.Encode(logger, rowsWithPk, 2, []int{0, 1}) c.Assert(err, ErrorMatches, "failed to cast value as bigint\\(20\\) for column `_tidb_rowid`.*Truncated.*") rowsWithPk2 := []types.Datum{ @@ -117,7 +117,7 @@ func (s *kvSuite) TestEncode(c *C) { Timestamp: 1234567891, }) c.Assert(err, IsNil) - pairs, err = mockMode.Encode(logger, rowsWithPk2, 2, []int{0, 1}) + _, err = mockMode.Encode(logger, rowsWithPk2, 2, []int{0, 1}) c.Assert(err, ErrorMatches, "mock error") // Non-strict mode @@ -214,9 +214,9 @@ func (s *kvSuite) TestEncodeTimestamp(c *C) { })) } -func mockTableInfo(c *C, createSql string) *model.TableInfo { +func mockTableInfo(c *C, createSQL string) *model.TableInfo { parser := parser.New() - node, err := parser.ParseOneStmt(createSql, "", "") + node, err := parser.ParseOneStmt(createSQL, "", "") c.Assert(err, IsNil) sctx := mock.NewContext() info, err := ddl.MockTableInfo(sctx, node.(*ast.CreateTableStmt), 1) @@ -398,6 +398,7 @@ func (s *benchSQL2KVSuite) SetUpTest(c *C) { tbl, err := tables.TableFromMeta(NewPanickingAllocators(0), tableInfo) c.Assert(err, IsNil) s.encoder, err = NewTableKVEncoder(tbl, &SessionOptions{SysVars: map[string]string{"tidb_row_format_version": "2"}}) + c.Assert(err, IsNil) s.logger = log.Logger{Logger: zap.NewNop()} // Prepare the row to insert. diff --git a/pkg/lightning/backend/local/local.go b/pkg/lightning/backend/local/local.go index 2eef4b447..0d6f48146 100644 --- a/pkg/lightning/backend/local/local.go +++ b/pkg/lightning/backend/local/local.go @@ -120,15 +120,14 @@ var ( // Range record start and end key for localStoreDir.DB // so we can write it to tikv in streaming type Range struct { - start []byte - end []byte - length int + start []byte + end []byte } // localFileMeta contains some field that is necessary to continue the engine restore/import process. // These field should be written to disk when we update chunk checkpoint type localFileMeta struct { - Ts uint64 `json:"ts"` + TS uint64 `json:"ts"` // Length is the number of KV pairs stored by the engine. Length atomic.Int64 `json:"length"` // TotalSize is the total pre-compressed KV byte size stored by engine. @@ -147,7 +146,7 @@ const ( type File struct { localFileMeta db *pebble.DB - Uuid uuid.UUID + UUID uuid.UUID localWriters sync.Map // isImportingAtomic is an atomic variable indicating whether this engine is importing. @@ -157,7 +156,7 @@ type File struct { } func (e *File) Close() error { - log.L().Debug("closing local engine", zap.Stringer("engine", e.Uuid), zap.Stack("stack")) + log.L().Debug("closing local engine", zap.Stringer("engine", e.UUID), zap.Stack("stack")) if e.db == nil { return nil } @@ -168,13 +167,13 @@ func (e *File) Close() error { // Cleanup remove meta and db files func (e *File) Cleanup(dataDir string) error { - dbPath := filepath.Join(dataDir, e.Uuid.String()) + dbPath := filepath.Join(dataDir, e.UUID.String()) return os.RemoveAll(dbPath) } // Exist checks if db folder existing (meta sometimes won't flush before lightning exit) func (e *File) Exist(dataDir string) error { - dbPath := filepath.Join(dataDir, e.Uuid.String()) + dbPath := filepath.Join(dataDir, e.UUID.String()) if _, err := os.Stat(dbPath); err != nil { return err } @@ -184,7 +183,7 @@ func (e *File) Exist(dataDir string) error { func (e *File) getSizeProperties() (*sizeProperties, error) { sstables, err := e.db.SSTables(pebble.WithProperties()) if err != nil { - log.L().Warn("get table properties failed", zap.Stringer("engine", e.Uuid), log.ShortError(err)) + log.L().Warn("get table properties failed", zap.Stringer("engine", e.UUID), log.ShortError(err)) return nil, errors.Trace(err) } @@ -195,7 +194,7 @@ func (e *File) getSizeProperties() (*sizeProperties, error) { data := hack.Slice(prop) rangeProps, err := decodeRangeProperties(data) if err != nil { - log.L().Warn("decodeRangeProperties failed", zap.Stringer("engine", e.Uuid), + log.L().Warn("decodeRangeProperties failed", zap.Stringer("engine", e.UUID), zap.Stringer("fileNum", info.FileNum), log.ShortError(err)) return nil, errors.Trace(err) } @@ -217,7 +216,7 @@ func (e *File) getEngineFileSize() backend.EngineFileSize { total := metrics.Total() var memSize int64 e.localWriters.Range(func(k, v interface{}) bool { - w := k.(*LocalWriter) + w := k.(*Writer) memSize += w.writeBatch.totalSize if w.writer != nil { total.Size += int64(w.writer.writer.EstimatedSize()) @@ -226,7 +225,7 @@ func (e *File) getEngineFileSize() backend.EngineFileSize { }) return backend.EngineFileSize{ - UUID: e.Uuid, + UUID: e.UUID, DiskSize: total.Size, MemSize: memSize, IsImporting: e.isLocked(), @@ -262,7 +261,7 @@ func (e *File) flushLocalWriters(parentCtx context.Context) error { eg, ctx := errgroup.WithContext(parentCtx) e.localWriters.Range(func(k, v interface{}) bool { eg.Go(func() error { - w := k.(*LocalWriter) + w := k.(*Writer) replyErrCh := make(chan error, 1) w.flushChMutex.RLock() if w.flushCh != nil { @@ -316,14 +315,14 @@ func (e *File) saveEngineMeta() error { func (e *File) loadEngineMeta() { jsonBytes, closer, err := e.db.Get(engineMetaKey) if err != nil { - log.L().Debug("local db missing engine meta", zap.Stringer("uuid", e.Uuid), zap.Error(err)) + log.L().Debug("local db missing engine meta", zap.Stringer("uuid", e.UUID), zap.Error(err)) return } defer closer.Close() err = json.Unmarshal(jsonBytes, &e.localFileMeta) if err != nil { - log.L().Warn("local db failed to deserialize meta", zap.Stringer("uuid", e.Uuid), zap.ByteString("content", jsonBytes), zap.Error(err)) + log.L().Warn("local db failed to deserialize meta", zap.Stringer("uuid", e.UUID), zap.ByteString("content", jsonBytes), zap.Error(err)) } } @@ -372,7 +371,6 @@ type connPool struct { mu sync.Mutex conns []*grpc.ClientConn - name string next int cap int newConn func(ctx context.Context) (*grpc.ClientConn, error) @@ -486,8 +484,8 @@ func NewLocalBackend( } // lock locks a local file and returns the File instance if it exists. -func (local *local) lockEngine(engineId uuid.UUID, state importMutexState) *File { - if e, ok := local.engines.Load(engineId); ok { +func (local *local) lockEngine(engineID uuid.UUID, state importMutexState) *File { + if e, ok := local.engines.Load(engineID); ok { engine := e.(*File) engine.lock(state) return engine @@ -576,13 +574,13 @@ func (local *local) Close() { } // FlushEngine ensure the written data is saved successfully, to make sure no data lose after restart -func (local *local) FlushEngine(ctx context.Context, engineId uuid.UUID) error { - engineFile := local.lockEngine(engineId, importMutexStateFlush) +func (local *local) FlushEngine(ctx context.Context, engineID uuid.UUID) error { + engineFile := local.lockEngine(engineID, importMutexStateFlush) // the engine cannot be deleted after while we've acquired the lock identified by UUID. if engineFile == nil { - return errors.Errorf("engine '%s' not found", engineId) + return errors.Errorf("engine '%s' not found", engineID) } defer engineFile.unlock() return engineFile.flushEngineWithoutLock(ctx) @@ -645,7 +643,7 @@ func (local *local) OpenEngine(ctx context.Context, engineUUID uuid.UUID) error if err != nil { return err } - e, _ := local.engines.LoadOrStore(engineUUID, &File{Uuid: engineUUID}) + e, _ := local.engines.LoadOrStore(engineUUID, &File{UUID: engineUUID}) engine := e.(*File) engine.db = db engine.loadEngineMeta() @@ -670,7 +668,7 @@ func (local *local) CloseEngine(ctx context.Context, engineUUID uuid.UUID) error return err } engineFile := &File{ - Uuid: engineUUID, + UUID: engineUUID, db: db, } engineFile.loadEngineMeta() @@ -770,7 +768,7 @@ func (local *local) WriteToTiKV( } req.Chunk = &sst.WriteRequest_Batch{ Batch: &sst.WriteBatch{ - CommitTs: engineFile.Ts, + CommitTs: engineFile.TS, }, } clients = append(clients, wstream) @@ -835,11 +833,9 @@ func (local *local) WriteToTiKV( for i, wStream := range clients { if resp, closeErr := wStream.CloseAndRecv(); closeErr != nil { return nil, nil, stats, closeErr - } else { - if leaderID == region.Region.Peers[i].GetId() { - leaderPeerMetas = resp.Metas - log.L().Debug("get metas after write kv stream to tikv", zap.Reflect("metas", leaderPeerMetas)) - } + } else if leaderID == region.Region.Peers[i].GetId() { + leaderPeerMetas = resp.Metas + log.L().Debug("get metas after write kv stream to tikv", zap.Reflect("metas", leaderPeerMetas)) } } @@ -955,7 +951,7 @@ func (local *local) readAndSplitIntoRange(engineFile *File) ([]Range, error) { // <= 96MB no need to split into range if engineFileTotalSize <= local.regionSplitSize && engineFileLength <= regionMaxKeyCount { - ranges := []Range{{start: firstKey, end: endKey, length: int(engineFileLength)}} + ranges := []Range{{start: firstKey, end: endKey}} return ranges, nil } @@ -967,7 +963,7 @@ func (local *local) readAndSplitIntoRange(engineFile *File) ([]Range, error) { ranges := splitRangeBySizeProps(Range{start: firstKey, end: endKey}, sizeProps, local.regionSplitSize, regionMaxKeyCount*2/3) - log.L().Info("split engine key ranges", zap.Stringer("engine", engineFile.Uuid), + log.L().Info("split engine key ranges", zap.Stringer("engine", engineFile.UUID), zap.Int64("totalSize", engineFileTotalSize), zap.Int64("totalCount", engineFileLength), log.ZapRedactBinary("firstKey", firstKey), log.ZapRedactBinary("lastKey", lastKey), zap.Int("ranges", len(ranges))) @@ -984,12 +980,8 @@ type bytesRecycleChan struct { // NOTE: we don't used a `sync.Pool` because when will sync.Pool release is depending on the // garbage collector which always release the memory so late. Use a fixed size chan to reuse // can decrease the memory usage to 1/3 compare with sync.Pool. -var recycleChan *bytesRecycleChan - -func init() { - recycleChan = &bytesRecycleChan{ - ch: make(chan []byte, 1024), - } +var recycleChan = &bytesRecycleChan{ + ch: make(chan []byte, 1024), } func (c *bytesRecycleChan) Acquire() []byte { @@ -1024,7 +1016,7 @@ func newBytesBuffer() *bytesBuffer { func (b *bytesBuffer) addBuf() { if b.curBufIdx < len(b.bufs)-1 { - b.curBufIdx += 1 + b.curBufIdx++ b.curBuf = b.bufs[b.curBufIdx] } else { buf := recycleChan.Acquire() @@ -1275,7 +1267,7 @@ loopWrite: func (local *local) writeAndIngestByRanges(ctx context.Context, engineFile *File, ranges []Range, remainRanges *syncdRanges) error { if engineFile.Length.Load() == 0 { // engine is empty, this is likes because it's a index engine but the table contains no index - log.L().Info("engine contains no data", zap.Stringer("uuid", engineFile.Uuid)) + log.L().Info("engine contains no data", zap.Stringer("uuid", engineFile.UUID)) return nil } log.L().Debug("the ranges Length write to tikv", zap.Int("Length", len(ranges))) @@ -1574,8 +1566,8 @@ func (local *local) LocalWriter(ctx context.Context, engineUUID uuid.UUID) (back return openLocalWriter(engineFile, local.localStoreDir, local.localWriterMemCacheSize), nil } -func openLocalWriter(f *File, sstDir string, memtableSizeLimit int64) *LocalWriter { - w := &LocalWriter{ +func openLocalWriter(f *File, sstDir string, memtableSizeLimit int64) *Writer { + w := &Writer{ sstDir: sstDir, kvsChan: make(chan []common.KvPair, defaultLocalWriterKVsChannelCap), flushCh: make(chan chan error), @@ -1808,7 +1800,7 @@ func (c *RangePropertiesCollector) insertNewPoint(key []byte) { // implement `TablePropertyCollector.Add` func (c *RangePropertiesCollector) Add(key pebble.InternalKey, value []byte) error { c.currentOffsets.Size += uint64(len(value)) + uint64(len(key.UserKey)) - c.currentOffsets.Keys += 1 + c.currentOffsets.Keys++ if len(c.lastKey) == 0 || c.sizeInLastRange() >= c.propSizeIdxDistance || c.keysInLastRange() >= c.propKeysIdxDistance { c.insertNewPoint(key.UserKey) @@ -1879,7 +1871,7 @@ func (local *local) EngineFileSizes() (res []backend.EngineFileSize) { return } -type LocalWriter struct { +type Writer struct { writeErr common.OnceError local *File consumeCh chan struct{} @@ -1893,7 +1885,7 @@ type LocalWriter struct { } // TODO: temporarily replace this async append rows with the former write-batch approach before addressing the performance issue. -func (w *LocalWriter) AppendRowsAsync(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { +func (w *Writer) AppendRowsAsync(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { kvs := kv.KvPairsFromRows(rows) if len(kvs) == 0 { return nil @@ -1902,12 +1894,12 @@ func (w *LocalWriter) AppendRowsAsync(ctx context.Context, tableName string, col return err } w.kvsChan <- kvs - w.local.Ts = ts + w.local.TS = ts return nil } // TODO: replace the implementation back with `AppendRowsAsync` after addressing the performance issue. -func (w *LocalWriter) AppendRows(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { +func (w *Writer) AppendRows(ctx context.Context, tableName string, columnNames []string, ts uint64, rows kv.Rows) error { kvs := kv.KvPairsFromRows(rows) if len(kvs) == 0 { return nil @@ -1924,12 +1916,12 @@ func (w *LocalWriter) AppendRows(ctx context.Context, tableName string, columnNa if err == nil { w.local.Length.Add(int64(len(kvs))) w.local.TotalSize.Add(int64(size)) - w.local.Ts = ts + w.local.TS = ts } return errors.Trace(err) } -func (w *LocalWriter) Close() error { +func (w *Writer) Close() error { w.local.localWriters.Delete(w) close(w.kvsChan) @@ -1951,11 +1943,11 @@ func (w *LocalWriter) Close() error { } } -func (w *LocalWriter) genSSTPath() string { +func (w *Writer) genSSTPath() string { return filepath.Join(w.sstDir, uuid.New().String()+".sst") } -func (w *LocalWriter) writeRowsLoop() { +func (w *Writer) writeRowsLoop() { defer func() { if w.writer != nil { w.writer.cleanUp() @@ -2020,7 +2012,7 @@ outside: } } -func (w *LocalWriter) writeKVsOrIngest(desc localIngestDescription) error { +func (w *Writer) writeKVsOrIngest(desc localIngestDescription) error { if w.writer != nil { if err := w.writer.writeKVs(&w.writeBatch); err != errorUnorderedSSTInsertion { return err diff --git a/pkg/lightning/backend/local/local_test.go b/pkg/lightning/backend/local/local_test.go index 22fe2e08d..58ec837b1 100644 --- a/pkg/lightning/backend/local/local_test.go +++ b/pkg/lightning/backend/local/local_test.go @@ -297,7 +297,7 @@ func testLocalWriter(c *C, needSort bool, partitialSort bool) { c.Assert(err, IsNil) meta := localFileMeta{} _, engineUUID := backend.MakeUUID("ww", 0) - f := File{localFileMeta: meta, db: db, Uuid: engineUUID} + f := File{localFileMeta: meta, db: db, UUID: engineUUID} w := openLocalWriter(&f, tmpPath, 1024*1024) ctx := context.Background() @@ -449,11 +449,12 @@ func (s *localSuite) TestIsIngestRetryable(c *C) { c.Assert(err, NotNil) resp.Error = &errorpb.Error{Message: "raft: proposal dropped"} - retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, meta) + retryType, _, err = local.isIngestRetryable(ctx, resp, region, meta) c.Assert(retryType, Equals, retryWrite) + c.Assert(err, NotNil) resp.Error = &errorpb.Error{Message: "unknown error"} - retryType, newRegion, err = local.isIngestRetryable(ctx, resp, region, meta) + retryType, _, err = local.isIngestRetryable(ctx, resp, region, meta) c.Assert(retryType, Equals, retryNone) c.Assert(err, ErrorMatches, "non-retryable error: unknown error") } diff --git a/pkg/lightning/backend/local/localhelper_test.go b/pkg/lightning/backend/local/localhelper_test.go index f8093302f..36abea79e 100644 --- a/pkg/lightning/backend/local/localhelper_test.go +++ b/pkg/lightning/backend/local/localhelper_test.go @@ -355,7 +355,7 @@ func (h *noopHook) AfterScanRegions(res []*restore.RegionInfo, err error) ([]*re return res, err } -func (s *localSuite) doTestBatchSplitRegionByRanges(c *C, ctx context.Context, hook clientHook, errPat string) { +func (s *localSuite) doTestBatchSplitRegionByRanges(ctx context.Context, c *C, hook clientHook, errPat string) { oldLimit := maxBatchSplitKeys oldSplitBackoffTime := splitRegionBaseBackOffTime maxBatchSplitKeys = 4 @@ -422,7 +422,7 @@ func (s *localSuite) doTestBatchSplitRegionByRanges(c *C, ctx context.Context, h } func (s *localSuite) TestBatchSplitRegionByRanges(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), nil, "") + s.doTestBatchSplitRegionByRanges(context.Background(), c, nil, "") } type scanRegionEmptyHook struct { @@ -440,7 +440,7 @@ func (h *scanRegionEmptyHook) AfterScanRegions(res []*restore.RegionInfo, err er } func (s *localSuite) TestBatchSplitRegionByRangesScanFailed(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), &scanRegionEmptyHook{}, "paginate scan region returns empty result") + s.doTestBatchSplitRegionByRanges(context.Background(), c, &scanRegionEmptyHook{}, "paginate scan region returns empty result") } type splitRegionEpochNotMatchHook struct { @@ -456,7 +456,7 @@ func (h *splitRegionEpochNotMatchHook) BeforeSplitRegion(ctx context.Context, re } func (s *localSuite) TestBatchSplitByRangesEpochNotMatch(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), &splitRegionEpochNotMatchHook{}, "batch split regions failed: epoch not match.*") + s.doTestBatchSplitRegionByRanges(context.Background(), c, &splitRegionEpochNotMatchHook{}, "batch split regions failed: epoch not match.*") } // return epoch not match error in every other call @@ -478,7 +478,7 @@ func (h *splitRegionEpochNotMatchHookRandom) BeforeSplitRegion(ctx context.Conte } func (s *localSuite) TestBatchSplitByRangesEpochNotMatchOnce(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), &splitRegionEpochNotMatchHookRandom{}, "") + s.doTestBatchSplitRegionByRanges(context.Background(), c, &splitRegionEpochNotMatchHookRandom{}, "") } type splitRegionNoValidKeyHook struct { @@ -497,11 +497,11 @@ func (h splitRegionNoValidKeyHook) BeforeSplitRegion(ctx context.Context, region } func (s *localSuite) TestBatchSplitByRangesNoValidKeysOnce(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), &splitRegionNoValidKeyHook{returnErrTimes: 1}, ".*no valid key.*") + s.doTestBatchSplitRegionByRanges(context.Background(), c, &splitRegionNoValidKeyHook{returnErrTimes: 1}, ".*no valid key.*") } func (s *localSuite) TestBatchSplitByRangesNoValidKeys(c *C) { - s.doTestBatchSplitRegionByRanges(c, context.Background(), &splitRegionNoValidKeyHook{returnErrTimes: math.MaxInt32}, ".*no valid key.*") + s.doTestBatchSplitRegionByRanges(context.Background(), c, &splitRegionNoValidKeyHook{returnErrTimes: math.MaxInt32}, ".*no valid key.*") } type reportAfterSplitHook struct { @@ -528,25 +528,25 @@ func (s *localSuite) TestBatchSplitByRangeCtxCanceled(c *C) { } }() - s.doTestBatchSplitRegionByRanges(c, ctx, &reportAfterSplitHook{ch: ch}, ".*context canceled.*") + s.doTestBatchSplitRegionByRanges(ctx, c, &reportAfterSplitHook{ch: ch}, ".*context canceled.*") close(ch) } func (s *localSuite) TestNeedSplit(c *C) { - tableId := int64(1) + tableID := int64(1) peers := make([]*metapb.Peer, 1) peers[0] = &metapb.Peer{ Id: 1, StoreId: 1, } keys := []int64{10, 100, 500, 1000, 999999, -1} - start := tablecodec.EncodeRowKeyWithHandle(tableId, 0) + start := tablecodec.EncodeRowKeyWithHandle(tableID, 0) regionStart := codec.EncodeBytes([]byte{}, start) regions := make([]*restore.RegionInfo, 0) for _, end := range keys { var regionEndKey []byte if end >= 0 { - endKey := tablecodec.EncodeRowKeyWithHandle(tableId, end) + endKey := tablecodec.EncodeRowKeyWithHandle(tableID, end) regionEndKey = codec.EncodeBytes([]byte{}, endKey) } region := &restore.RegionInfo{ @@ -574,7 +574,7 @@ func (s *localSuite) TestNeedSplit(c *C) { } for hdl, idx := range checkMap { - checkKey := tablecodec.EncodeRowKeyWithHandle(tableId, hdl) + checkKey := tablecodec.EncodeRowKeyWithHandle(tableID, hdl) res := needSplit(checkKey, regions) if idx < 0 { c.Assert(res, IsNil) diff --git a/pkg/lightning/backend/tidb/tidb.go b/pkg/lightning/backend/tidb/tidb.go index 6ef8ca489..90a7d9276 100644 --- a/pkg/lightning/backend/tidb/tidb.go +++ b/pkg/lightning/backend/tidb/tidb.go @@ -57,8 +57,8 @@ type tidbRow string type tidbRows []tidbRow // MarshalLogArray implements the zapcore.ArrayMarshaler interface -func (row tidbRows) MarshalLogArray(encoder zapcore.ArrayEncoder) error { - for _, r := range row { +func (rows tidbRows) MarshalLogArray(encoder zapcore.ArrayEncoder) error { + for _, r := range rows { encoder.AppendString(string(r)) } return nil @@ -95,7 +95,9 @@ func NewTiDBBackend(db *sql.DB, onDuplicate string) backend.Backend { func (row tidbRow) ClassifyAndAppend(data *kv.Rows, checksum *verification.KVChecksum, _ *kv.Rows, _ *verification.KVChecksum) { rows := (*data).(tidbRows) - *data = tidbRows(append(rows, row)) + // Cannot do `rows := data.(*tidbRows); *rows = append(*rows, row)`. + //nolint:gocritic + *data = append(rows, row) cs := verification.MakeKVChecksum(uint64(len(row)), 1, 0) checksum.Add(&cs) } @@ -165,7 +167,7 @@ func (enc *tidbEncoder) appendSQLBytes(sb *strings.Builder, value []byte) { // appendSQL appends the SQL representation of the Datum into the string builder. // Note that we cannot use Datum.ToString since it doesn't perform SQL escaping. -func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, col *table.Column) error { +func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, _ *table.Column) error { switch datum.Kind() { case types.KindNull: sb.WriteString("NULL") @@ -195,13 +197,13 @@ func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, col * sb.Write(value) case types.KindString: // See: https://github.com/pingcap/tidb-lightning/issues/550 - //if enc.mode.HasStrictMode() { + // if enc.mode.HasStrictMode() { // d, err := table.CastValue(enc.se, *datum, col.ToInfo(), false, false) // if err != nil { // return errors.Trace(err) // } // datum = &d - //} + // } enc.appendSQLBytes(sb, datum.GetBytes()) case types.KindBytes: @@ -218,7 +220,9 @@ func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, col * value := datum.GetBinaryLiteral() sb.Grow(3 + 2*len(value)) sb.WriteString("x'") - hex.NewEncoder(sb).Write(value) + if _, err := hex.NewEncoder(sb).Write(value); err != nil { + return errors.Trace(err) + } sb.WriteByte('\'') case types.KindMysqlBit: @@ -285,7 +289,8 @@ func (enc *tidbEncoder) Encode(logger log.Logger, row []types.Datum, _ int64, co if i != 0 { encoded.WriteByte(',') } - if err := enc.appendSQL(&encoded, &field, getColumnByIndex(cols, enc.columnIdx[i])); err != nil { + datum := field + if err := enc.appendSQL(&encoded, &datum, getColumnByIndex(cols, enc.columnIdx[i])); err != nil { logger.Error("tidb encode failed", zap.Array("original", kv.RowArrayMarshaler(row)), zap.Int("originalCol", i), @@ -423,6 +428,7 @@ func (be *tidbBackend) WriteRowsToDB(ctx context.Context, tableName string, colu return errors.Trace(err) } +//nolint:nakedret // TODO: refactor func (be *tidbBackend) FetchRemoteTableModels(ctx context.Context, schemaName string) (tables []*model.TableInfo, err error) { s := common.SQLWithRetry{ DB: be.db, @@ -513,11 +519,11 @@ func (be *tidbBackend) FetchRemoteTableModels(ctx context.Context, schemaName st return err } - //+--------------+------------+-------------+--------------------+----------------+ - //| DB_NAME | TABLE_NAME | COLUMN_NAME | NEXT_GLOBAL_ROW_ID | ID_TYPE | - //+--------------+------------+-------------+--------------------+----------------+ - //| testsysbench | t | _tidb_rowid | 1 | AUTO_INCREMENT | - //+--------------+------------+-------------+--------------------+----------------+ + // +--------------+------------+-------------+--------------------+----------------+ + // | DB_NAME | TABLE_NAME | COLUMN_NAME | NEXT_GLOBAL_ROW_ID | ID_TYPE | + // +--------------+------------+-------------+--------------------+----------------+ + // | testsysbench | t | _tidb_rowid | 1 | AUTO_INCREMENT | + // +--------------+------------+-------------+--------------------+----------------+ // if columns length is 4, it doesn't contains the last column `ID_TYPE`, and it will always be 'AUTO_INCREMENT' // for v4.0.0~v4.0.2 show table t next_row_id only returns 4 columns. @@ -545,9 +551,13 @@ func (be *tidbBackend) FetchRemoteTableModels(ctx context.Context, schemaName st } } } - rows.Close() + // Defer in for-loop would be costly, anyway, we don't need those rows after this turn of iteration. + //nolint:sqlclosecheck + if err := rows.Close(); err != nil { + return errors.Trace(err) + } if rows.Err() != nil { - return rows.Err() + return errors.Trace(rows.Err()) } } return nil @@ -572,18 +582,18 @@ func (be *tidbBackend) ResetEngine(context.Context, uuid.UUID) error { } func (be *tidbBackend) LocalWriter(ctx context.Context, engineUUID uuid.UUID) (backend.EngineWriter, error) { - return &TiDBWriter{be: be, engineUUID: engineUUID}, nil + return &Writer{be: be, engineUUID: engineUUID}, nil } -type TiDBWriter struct { +type Writer struct { be *tidbBackend engineUUID uuid.UUID } -func (w *TiDBWriter) Close() error { +func (w *Writer) Close() error { return nil } -func (w *TiDBWriter) AppendRows(ctx context.Context, tableName string, columnNames []string, arg1 uint64, rows kv.Rows) error { +func (w *Writer) AppendRows(ctx context.Context, tableName string, columnNames []string, arg1 uint64, rows kv.Rows) error { return w.be.WriteRows(ctx, w.engineUUID, tableName, columnNames, arg1, rows) } diff --git a/pkg/lightning/backend/tidb/tidb_test.go b/pkg/lightning/backend/tidb/tidb_test.go index 785e1b9dc..d8f48303d 100644 --- a/pkg/lightning/backend/tidb/tidb_test.go +++ b/pkg/lightning/backend/tidb/tidb_test.go @@ -163,11 +163,13 @@ func (s *mysqlSuite) TestWriteRowsIgnoreOnDup(c *C) { // test encode rows with _tidb_rowid encoder, err = ignoreBackend.NewEncoder(s.tbl, &kv.SessionOptions{}) c.Assert(err, IsNil) - row, err = encoder.Encode(logger, []types.Datum{ + rowWithID, err := encoder.Encode(logger, []types.Datum{ types.NewIntDatum(1), types.NewIntDatum(1), // _tidb_rowid field }, 1, []int{0, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, 1}) c.Assert(err, IsNil) + // tidbRow is string. + c.Assert(fmt.Sprint(rowWithID), Equals, "(1,1)") } func (s *mysqlSuite) TestWriteRowsErrorOnDup(c *C) { @@ -205,6 +207,7 @@ func (s *mysqlSuite) TestWriteRowsErrorOnDup(c *C) { } // TODO: temporarily disable this test before we fix strict mode +//nolint:unused func (s *mysqlSuite) testStrictMode(c *C) { ft := *types.NewFieldType(mysql.TypeVarchar) ft.Charset = charset.CharsetUTF8MB4 diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index 67a0fdb43..70c9053bb 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -66,6 +66,11 @@ const ( CheckpointTableNameTable = "table_v6" CheckpointTableNameEngine = "engine_v5" CheckpointTableNameChunk = "chunk_v5" + + // Some frequently used table name or constants. + allTables = "all" + stringLitAll = "'all'" + columnTableName = "table_name" ) const ( @@ -361,14 +366,15 @@ func (cp *TableCheckpoint) Apply(cpd *TableCheckpointDiff) { engine.Status = engineDiff.status } for key, diff := range engineDiff.chunks { + checkpointKey := key index := sort.Search(len(engine.Chunks), func(i int) bool { - return !engine.Chunks[i].Key.less(&key) + return !engine.Chunks[i].Key.less(&checkpointKey) }) if index >= len(engine.Chunks) { continue } chunk := engine.Chunks[index] - if chunk.Key != key { + if chunk.Key != checkpointKey { continue } chunk.Chunk.Offset = diff.pos @@ -448,7 +454,7 @@ type DestroyedTableCheckpoint struct { } type TaskCheckpoint struct { - TaskId int64 + TaskID int64 SourceDir string Backend string ImporterAddr string @@ -459,7 +465,7 @@ type TaskCheckpoint struct { LightningVer string } -type CheckpointsDB interface { +type DB interface { Initialize(ctx context.Context, cfg *config.Config, dbInfo map[string]*TidbDBInfo) error TaskCheckpoint(ctx context.Context) (*TaskCheckpoint, error) Get(ctx context.Context, tableName string) (*TableCheckpoint, error) @@ -484,7 +490,7 @@ type CheckpointsDB interface { DumpChunks(ctx context.Context, csv io.Writer) error } -func OpenCheckpointsDB(ctx context.Context, cfg *config.Config) (CheckpointsDB, error) { +func OpenCheckpointsDB(ctx context.Context, cfg *config.Config) (DB, error) { if !cfg.Checkpoint.Enable { return NewNullCheckpointsDB(), nil } @@ -527,7 +533,11 @@ func IsCheckpointsDBExists(ctx context.Context, cfg *config.Config) (bool, error return false, errors.Trace(err) } defer rows.Close() - return rows.Next(), nil + result := rows.Next() + if err := rows.Err(); err != nil { + return false, errors.Trace(err) + } + return result, nil case config.CheckpointDriverFile: _, err := os.Stat(cfg.Checkpoint.DSN) @@ -677,7 +687,7 @@ func (cpdb *MySQLCheckpointsDB) TaskCheckpoint(ctx context.Context) (*TaskCheckp taskQuery := fmt.Sprintf(ReadTaskTemplate, cpdb.schema, CheckpointTableNameTask) taskCp := &TaskCheckpoint{} - err := s.QueryRow(ctx, "fetch task checkpoint", taskQuery, &taskCp.TaskId, &taskCp.SourceDir, &taskCp.Backend, + err := s.QueryRow(ctx, "fetch task checkpoint", taskQuery, &taskCp.TaskID, &taskCp.SourceDir, &taskCp.Backend, &taskCp.ImporterAddr, &taskCp.TiDBHost, &taskCp.TiDBPort, &taskCp.PdAddr, &taskCp.SortedKVDir, &taskCp.LightningVer) if err != nil { // if task checkpoint is empty, return nil @@ -1011,7 +1021,7 @@ func (cpdb *FileCheckpointsDB) TaskCheckpoint(_ context.Context) (*TaskCheckpoin } return &TaskCheckpoint{ - TaskId: cp.TaskId, + TaskID: cp.TaskId, SourceDir: cp.SourceDir, Backend: cp.Backend, ImporterAddr: cp.ImporterAddr, @@ -1123,7 +1133,6 @@ func (cpdb *FileCheckpointsDB) InsertEngineCheckpoints(_ context.Context, tableN if len(value.ColumnPermutation) > 0 { chunk.ColumnPermutation = intSlice2Int32Slice(value.ColumnPermutation) } - } tableModel.Engines[engineID] = engineModel } @@ -1168,14 +1177,14 @@ func (cpdb *FileCheckpointsDB) Update(checkpointDiffs map[string]*TableCheckpoin // Management functions ---------------------------------------------------------------------------- -var cannotManageNullDB = errors.New("cannot perform this function while checkpoints is disabled") +var errCannotManageNullDB = errors.New("cannot perform this function while checkpoints is disabled") func (*NullCheckpointsDB) RemoveCheckpoint(context.Context, string) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) MoveCheckpoints(context.Context, int64) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) GetLocalStoringTables(context.Context) (map[string][]int32, error) { @@ -1183,23 +1192,23 @@ func (*NullCheckpointsDB) GetLocalStoringTables(context.Context) (map[string][]i } func (*NullCheckpointsDB) IgnoreErrorCheckpoint(context.Context, string) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) DestroyErrorCheckpoint(context.Context, string) ([]DestroyedTableCheckpoint, error) { - return nil, errors.Trace(cannotManageNullDB) + return nil, errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) DumpTables(context.Context, io.Writer) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) DumpEngines(context.Context, io.Writer) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (*NullCheckpointsDB) DumpChunks(context.Context, io.Writer) error { - return errors.Trace(cannotManageNullDB) + return errors.Trace(errCannotManageNullDB) } func (cpdb *MySQLCheckpointsDB) RemoveCheckpoint(ctx context.Context, tableName string) error { @@ -1208,7 +1217,7 @@ func (cpdb *MySQLCheckpointsDB) RemoveCheckpoint(ctx context.Context, tableName Logger: log.With(zap.String("table", tableName)), } - if tableName == "all" { + if tableName == allTables { return s.Exec(ctx, "remove all checkpoints", "DROP SCHEMA "+cpdb.schema) } @@ -1307,12 +1316,12 @@ func (cpdb *MySQLCheckpointsDB) GetLocalStoringTables(ctx context.Context) (map[ func (cpdb *MySQLCheckpointsDB) IgnoreErrorCheckpoint(ctx context.Context, tableName string) error { var colName string - if tableName == "all" { + if tableName == allTables { // This will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" + colName = stringLitAll } else { - colName = "table_name" + colName = columnTableName } engineQuery := fmt.Sprintf(` @@ -1341,13 +1350,13 @@ func (cpdb *MySQLCheckpointsDB) IgnoreErrorCheckpoint(ctx context.Context, table func (cpdb *MySQLCheckpointsDB) DestroyErrorCheckpoint(ctx context.Context, tableName string) ([]DestroyedTableCheckpoint, error) { var colName, aliasedColName string - if tableName == "all" { + if tableName == allTables { // These will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" - aliasedColName = "'all'" + colName = stringLitAll + aliasedColName = stringLitAll } else { - colName = "table_name" + colName = columnTableName aliasedColName = "t.table_name" } @@ -1415,6 +1424,7 @@ func (cpdb *MySQLCheckpointsDB) DestroyErrorCheckpoint(ctx context.Context, tabl return targetTables, nil } +//nolint:rowserrcheck // sqltocsv.Write will check this. func (cpdb *MySQLCheckpointsDB) DumpTables(ctx context.Context, writer io.Writer) error { rows, err := cpdb.db.QueryContext(ctx, fmt.Sprintf(` SELECT @@ -1435,6 +1445,7 @@ func (cpdb *MySQLCheckpointsDB) DumpTables(ctx context.Context, writer io.Writer return errors.Trace(sqltocsv.Write(writer, rows)) } +//nolint:rowserrcheck // sqltocsv.Write will check this. func (cpdb *MySQLCheckpointsDB) DumpEngines(ctx context.Context, writer io.Writer) error { rows, err := cpdb.db.QueryContext(ctx, fmt.Sprintf(` SELECT @@ -1453,6 +1464,7 @@ func (cpdb *MySQLCheckpointsDB) DumpEngines(ctx context.Context, writer io.Write return errors.Trace(sqltocsv.Write(writer, rows)) } +//nolint:rowserrcheck // sqltocsv.Write will check this. func (cpdb *MySQLCheckpointsDB) DumpChunks(ctx context.Context, writer io.Writer) error { rows, err := cpdb.db.QueryContext(ctx, fmt.Sprintf(` SELECT @@ -1487,7 +1499,7 @@ func (cpdb *FileCheckpointsDB) RemoveCheckpoint(_ context.Context, tableName str cpdb.lock.Lock() defer cpdb.lock.Unlock() - if tableName == "all" { + if tableName == allTables { cpdb.checkpoints.Reset() return errors.Trace(os.Remove(cpdb.path)) } @@ -1527,7 +1539,6 @@ func (cpdb *FileCheckpointsDB) GetLocalStoringTables(_ context.Context) (map[str break } } - } } @@ -1539,7 +1550,7 @@ func (cpdb *FileCheckpointsDB) IgnoreErrorCheckpoint(_ context.Context, targetTa defer cpdb.lock.Unlock() for tableName, tableModel := range cpdb.checkpoints.Checkpoints { - if !(targetTableName == "all" || targetTableName == tableName) { + if !(targetTableName == allTables || targetTableName == tableName) { continue } if tableModel.Status <= uint32(CheckpointStatusMaxInvalid) { @@ -1562,7 +1573,7 @@ func (cpdb *FileCheckpointsDB) DestroyErrorCheckpoint(_ context.Context, targetT for tableName, tableModel := range cpdb.checkpoints.Checkpoints { // Obtain the list of tables - if !(targetTableName == "all" || targetTableName == tableName) { + if !(targetTableName == allTables || targetTableName == tableName) { continue } if tableModel.Status <= uint32(CheckpointStatusMaxInvalid) { diff --git a/pkg/lightning/checkpoints/checkpoints_file_test.go b/pkg/lightning/checkpoints/checkpoints_file_test.go index c803395ae..32dfc3647 100644 --- a/pkg/lightning/checkpoints/checkpoints_file_test.go +++ b/pkg/lightning/checkpoints/checkpoints_file_test.go @@ -22,9 +22,7 @@ func Test(t *testing.T) { var _ = Suite(&cpFileSuite{}) type cpFileSuite struct { - path string cpdb *checkpoints.FileCheckpointsDB - cfg *config.Config } func newTestConfig() *config.Config { diff --git a/pkg/lightning/checkpoints/glue_checkpoint.go b/pkg/lightning/checkpoints/glue_checkpoint.go index 82c3bc5b0..5eb869c20 100644 --- a/pkg/lightning/checkpoints/glue_checkpoint.go +++ b/pkg/lightning/checkpoints/glue_checkpoint.go @@ -55,7 +55,15 @@ type GlueCheckpointsDB struct { schema string } -var _ CheckpointsDB = (*GlueCheckpointsDB)(nil) +var _ DB = (*GlueCheckpointsDB)(nil) + +// dropPreparedStmt drops the statement and when meet an error, +// print an error message. +func dropPreparedStmt(session Session, stmtID uint32) { + if err := session.DropPreparedStmt(stmtID); err != nil { + log.L().Error("failed to drop prepared statement", log.ShortError(err)) + } +} func NewGlueCheckpointsDB(ctx context.Context, se Session, f func() (Session, error), schemaName string) (*GlueCheckpointsDB, error) { var escapedSchemaName strings.Builder @@ -127,7 +135,7 @@ func (g GlueCheckpointsDB) Initialize(ctx context.Context, cfg *config.Config, d if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(stmtID) + defer dropPreparedStmt(s, stmtID) _, err = s.ExecutePreparedStmt(c, stmtID, []types.Datum{ types.NewIntDatum(cfg.TaskID), types.NewStringDatum(cfg.Mydumper.SourceDir), @@ -147,7 +155,7 @@ func (g GlueCheckpointsDB) Initialize(ctx context.Context, cfg *config.Config, d if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(stmtID2) + defer dropPreparedStmt(s, stmtID2) for _, db := range dbInfo { for _, table := range db.Tables { @@ -196,7 +204,7 @@ func (g GlueCheckpointsDB) TaskCheckpoint(ctx context.Context) (*TaskCheckpoint, row := req.GetRow(0) taskCp = &TaskCheckpoint{} - taskCp.TaskId = row.GetInt64(0) + taskCp.TaskID = row.GetInt64(0) taskCp.SourceDir = row.GetString(1) taskCp.Backend = row.GetString(2) taskCp.ImporterAddr = row.GetString(3) @@ -355,13 +363,13 @@ func (g GlueCheckpointsDB) InsertEngineCheckpoints(ctx context.Context, tableNam if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(engineStmt) + defer dropPreparedStmt(s, engineStmt) chunkStmt, _, _, err := s.PrepareStmt(fmt.Sprintf(ReplaceChunkTemplate, g.schema, CheckpointTableNameChunk)) if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(chunkStmt) + defer dropPreparedStmt(s, chunkStmt) for engineID, engine := range checkpointMap { _, err := s.ExecutePreparedStmt(c, engineStmt, []types.Datum{ @@ -421,22 +429,22 @@ func (g GlueCheckpointsDB) Update(checkpointDiffs map[string]*TableCheckpointDif if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(chunkStmt) + defer dropPreparedStmt(s, chunkStmt) rebaseStmt, _, _, err := s.PrepareStmt(rebaseQuery) if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(rebaseStmt) + defer dropPreparedStmt(s, rebaseStmt) tableStatusStmt, _, _, err := s.PrepareStmt(tableStatusQuery) if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(tableStatusStmt) + defer dropPreparedStmt(s, tableStatusStmt) engineStatusStmt, _, _, err := s.PrepareStmt(engineStatusQuery) if err != nil { return errors.Trace(err) } - defer s.DropPreparedStmt(engineStatusStmt) + defer dropPreparedStmt(s, engineStatusStmt) for tableName, cpd := range checkpointDiffs { if cpd.hasStatus { @@ -506,7 +514,7 @@ func (g GlueCheckpointsDB) RemoveCheckpoint(ctx context.Context, tableName strin } defer se.Close() - if tableName == "all" { + if tableName == allTables { return common.Retry("remove all checkpoints", logger, func() error { _, err := se.Execute(ctx, "DROP SCHEMA "+g.schema) return err @@ -625,12 +633,12 @@ func (g GlueCheckpointsDB) IgnoreErrorCheckpoint(ctx context.Context, tableName defer se.Close() var colName string - if tableName == "all" { + if tableName == allTables { // This will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" + colName = stringLitAll } else { - colName = "table_name" + colName = columnTableName } tableName = common.InterpolateMySQLString(tableName) @@ -662,13 +670,13 @@ func (g GlueCheckpointsDB) DestroyErrorCheckpoint(ctx context.Context, tableName var colName, aliasedColName string - if tableName == "all" { + if tableName == allTables { // These will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" - aliasedColName = "'all'" + colName = stringLitAll + aliasedColName = stringLitAll } else { - colName = "table_name" + colName = columnTableName aliasedColName = "t.table_name" } diff --git a/pkg/lightning/common/pause_test.go b/pkg/lightning/common/pause_test.go index cb7d2264b..298e023cd 100644 --- a/pkg/lightning/common/pause_test.go +++ b/pkg/lightning/common/pause_test.go @@ -147,7 +147,7 @@ func (s *pauseSuite) BenchmarkWaitNoOp(c *C) { p := common.NewPauser() ctx := context.Background() for i := 0; i < c.N; i++ { - p.Wait(ctx) + _ = p.Wait(ctx) } } @@ -157,7 +157,7 @@ func (s *pauseSuite) BenchmarkWaitCtxCanceled(c *C) { ctx, cancel := context.WithCancel(context.Background()) cancel() for i := 0; i < c.N; i++ { - p.Wait(ctx) + _ = p.Wait(ctx) } } @@ -185,6 +185,6 @@ func (s *pauseSuite) BenchmarkWaitContended(c *C) { ctx := context.Background() for i := 0; i < c.N; i++ { - p.Wait(ctx) + _ = p.Wait(ctx) } } diff --git a/pkg/lightning/common/security_test.go b/pkg/lightning/common/security_test.go index e7db1f86d..bf9879abd 100644 --- a/pkg/lightning/common/security_test.go +++ b/pkg/lightning/common/security_test.go @@ -32,9 +32,9 @@ type securitySuite struct{} var _ = Suite(&securitySuite{}) func respondPathHandler(w http.ResponseWriter, req *http.Request) { - io.WriteString(w, `{"path":"`) - io.WriteString(w, req.URL.Path) - io.WriteString(w, `"}`) + _, _ = io.WriteString(w, `{"path":"`) + _, _ = io.WriteString(w, req.URL.Path) + _, _ = io.WriteString(w, `"}`) } func (s *securitySuite) TestGetJSONInsecure(c *C) { diff --git a/pkg/lightning/common/util.go b/pkg/lightning/common/util.go index b9d4013f7..f3b6f8059 100644 --- a/pkg/lightning/common/util.go +++ b/pkg/lightning/common/util.go @@ -106,7 +106,7 @@ type SQLWithRetry struct { HideQueryLog bool } -func (t SQLWithRetry) perform(ctx context.Context, parentLogger log.Logger, purpose string, action func() error) error { +func (t SQLWithRetry) perform(_ context.Context, parentLogger log.Logger, purpose string, action func() error) error { return Retry(purpose, parentLogger, action) } diff --git a/pkg/lightning/common/util_test.go b/pkg/lightning/common/util_test.go index 3fab74640..17998de20 100644 --- a/pkg/lightning/common/util_test.go +++ b/pkg/lightning/common/util_test.go @@ -57,7 +57,7 @@ func (s *utilSuite) TestGetJSON(c *C) { ctx := context.Background() // Mock success response - handle := func(res http.ResponseWriter, req *http.Request) { + handle := func(res http.ResponseWriter, _ *http.Request) { res.WriteHeader(http.StatusOK) err := json.NewEncoder(res).Encode(request) c.Assert(err, IsNil) @@ -77,7 +77,7 @@ func (s *utilSuite) TestGetJSON(c *C) { c.Assert(request, DeepEquals, response) // Mock `StatusNoContent` response - handle = func(res http.ResponseWriter, req *http.Request) { + handle = func(res http.ResponseWriter, _ *http.Request) { res.WriteHeader(http.StatusNoContent) } err = common.GetJSON(ctx, client, testServer.URL, &response) diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index f56f72c45..5a4969f8f 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -71,9 +71,6 @@ const ( defaultIndexSerialScanConcurrency = 20 defaultChecksumTableConcurrency = 2 - defaultEngineMemCacheSize = 512 * units.MiB - defaultLocalWriterMemCacheSize = 128 * units.MiB - // autoDiskQuotaLocalReservedSpeed is the estimated size increase per // millisecond per write thread the local backend may gain on all engines. // This is used to compute the maximum size overshoot between two disk quota @@ -82,10 +79,11 @@ const ( // With cron.check-disk-quota = 1m, region-concurrency = 40, this should // contribute 2.3 GiB to the reserved size. autoDiskQuotaLocalReservedSpeed uint64 = 1 * units.KiB + defaultEngineMemCacheSize = 512 * units.MiB + defaultLocalWriterMemCacheSize = 128 * units.MiB ) var ( - defaultConfigPaths = []string{"tidb-lightning.toml", "conf/tidb-lightning.toml"} supportedStorageTypes = []string{"file", "local", "s3", "noop"} DefaultFilter = []string{ @@ -136,17 +134,17 @@ type Config struct { BWList filter.MySQLReplicationRules `toml:"black-white-list" json:"black-white-list"` } -func (c *Config) String() string { - bytes, err := json.Marshal(c) +func (cfg *Config) String() string { + bytes, err := json.Marshal(cfg) if err != nil { log.L().Error("marshal config to json error", log.ShortError(err)) } return string(bytes) } -func (c *Config) ToTLS() (*common.TLS, error) { - hostPort := net.JoinHostPort(c.TiDB.Host, strconv.Itoa(c.TiDB.StatusPort)) - return common.NewTLS(c.Security.CAPath, c.Security.CertPath, c.Security.KeyPath, hostPort) +func (cfg *Config) ToTLS() (*common.TLS, error) { + hostPort := net.JoinHostPort(cfg.TiDB.Host, strconv.Itoa(cfg.TiDB.StatusPort)) + return common.NewTLS(cfg.Security.CAPath, cfg.Security.CertPath, cfg.Security.KeyPath, hostPort) } type Lightning struct { @@ -188,6 +186,7 @@ func (t PostOpLevel) MarshalText() ([]byte, error) { // parser command line parameter func (t *PostOpLevel) FromStringValue(s string) error { switch strings.ToLower(s) { + //nolint:goconst // This 'false' and other 'false's aren't the same. case "off", "false": *t = OpLevelOff case "required", "true": @@ -223,20 +222,20 @@ func (t PostOpLevel) String() string { // PostRestore has some options which will be executed after kv restored. type PostRestore struct { - Level1Compact bool `toml:"level-1-compact" json:"level-1-compact"` - Compact bool `toml:"compact" json:"compact"` Checksum PostOpLevel `toml:"checksum" json:"checksum"` Analyze PostOpLevel `toml:"analyze" json:"analyze"` + Level1Compact bool `toml:"level-1-compact" json:"level-1-compact"` PostProcessAtLast bool `toml:"post-process-at-last" json:"post-process-at-last"` + Compact bool `toml:"compact" json:"compact"` } type CSVConfig struct { Separator string `toml:"separator" json:"separator"` Delimiter string `toml:"delimiter" json:"delimiter"` + Null string `toml:"null" json:"null"` Header bool `toml:"header" json:"header"` TrimLastSep bool `toml:"trim-last-separator" json:"trim-last-separator"` NotNull bool `toml:"not-null" json:"not-null"` - Null string `toml:"null" json:"null"` BackslashEscape bool `toml:"backslash-escape" json:"backslash-escape"` } @@ -245,14 +244,14 @@ type MydumperRuntime struct { BatchSize ByteSize `toml:"batch-size" json:"batch-size"` BatchImportRatio float64 `toml:"batch-import-ratio" json:"batch-import-ratio"` SourceDir string `toml:"data-source-dir" json:"data-source-dir"` - NoSchema bool `toml:"no-schema" json:"no-schema"` CharacterSet string `toml:"character-set" json:"character-set"` CSV CSVConfig `toml:"csv" json:"csv"` - CaseSensitive bool `toml:"case-sensitive" json:"case-sensitive"` - StrictFormat bool `toml:"strict-format" json:"strict-format"` MaxRegionSize ByteSize `toml:"max-region-size" json:"max-region-size"` Filter []string `toml:"filter" json:"filter"` FileRouters []*FileRouteRule `toml:"files" json:"files"` + NoSchema bool `toml:"no-schema" json:"no-schema"` + CaseSensitive bool `toml:"case-sensitive" json:"case-sensitive"` + StrictFormat bool `toml:"strict-format" json:"strict-format"` DefaultFileRules bool `toml:"default-file-rules" json:"default-file-rules"` } @@ -282,10 +281,10 @@ type TikvImporter struct { } type Checkpoint struct { - Enable bool `toml:"enable" json:"enable"` Schema string `toml:"schema" json:"schema"` DSN string `toml:"dsn" json:"-"` // DSN may contain password, don't expose this to JSON. Driver string `toml:"driver" json:"driver"` + Enable bool `toml:"enable" json:"enable"` KeepAfterSuccess bool `toml:"keep-after-success" json:"keep-after-success"` } @@ -312,7 +311,7 @@ func (sec *Security) RegisterMySQL() error { tlsConfig, err := common.ToTLSConfig(sec.CAPath, sec.CertPath, sec.KeyPath) switch { case err != nil: - return err + return errors.Trace(err) case tlsConfig != nil: // error happens only when the key coincides with the built-in names. _ = gomysql.RegisterTLSConfig("cluster", tlsConfig) @@ -331,7 +330,7 @@ type Duration struct { func (d *Duration) UnmarshalText(text []byte) error { var err error d.Duration, err = time.ParseDuration(string(text)) - return err + return errors.Trace(err) } func (d Duration) MarshalText() ([]byte, error) { @@ -533,40 +532,12 @@ func (cfg *Config) Adjust(ctx context.Context) error { mustHaveInternalConnections := true switch cfg.TikvImporter.Backend { case BackendTiDB: - if cfg.App.IndexConcurrency == 0 { - cfg.App.IndexConcurrency = cfg.App.RegionConcurrency - } - if cfg.App.TableConcurrency == 0 { - cfg.App.TableConcurrency = cfg.App.RegionConcurrency - } + cfg.DefaultVarsForTiDBBackend() mustHaveInternalConnections = false cfg.PostRestore.Checksum = OpLevelOff cfg.PostRestore.Analyze = OpLevelOff case BackendImporter, BackendLocal: - if cfg.App.IndexConcurrency == 0 { - cfg.App.IndexConcurrency = 2 - } - if cfg.App.TableConcurrency == 0 { - cfg.App.TableConcurrency = 6 - } - if cfg.TikvImporter.RangeConcurrency == 0 { - cfg.TikvImporter.RangeConcurrency = 16 - } - if cfg.TikvImporter.RegionSplitSize == 0 { - cfg.TikvImporter.RegionSplitSize = SplitRegionSize - } - if cfg.TiDB.DistSQLScanConcurrency == 0 { - cfg.TiDB.DistSQLScanConcurrency = defaultDistSQLScanConcurrency - } - if cfg.TiDB.BuildStatsConcurrency == 0 { - cfg.TiDB.BuildStatsConcurrency = defaultBuildStatsConcurrency - } - if cfg.TiDB.IndexSerialScanConcurrency == 0 { - cfg.TiDB.IndexSerialScanConcurrency = defaultIndexSerialScanConcurrency - } - if cfg.TiDB.ChecksumTableConcurrency == 0 { - cfg.TiDB.ChecksumTableConcurrency = defaultChecksumTableConcurrency - } + cfg.DefaultVarsForImporterAndLocalBackend() default: return errors.Errorf("invalid config: unsupported `tikv-importer.backend` (%s)", cfg.TikvImporter.Backend) } @@ -580,42 +551,8 @@ func (cfg *Config) Adjust(ctx context.Context) error { } if cfg.TikvImporter.Backend == BackendLocal { - if len(cfg.TikvImporter.SortedKVDir) == 0 { - return errors.Errorf("tikv-importer.sorted-kv-dir must not be empty!") - } - - storageSizeDir := filepath.Clean(cfg.TikvImporter.SortedKVDir) - sortedKVDirInfo, err := os.Stat(storageSizeDir) - switch { - case os.IsNotExist(err): - // the sorted-kv-dir does not exist, meaning we will create it automatically. - // so we extract the storage size from its parent directory. - storageSizeDir = filepath.Dir(storageSizeDir) - case err == nil: - if !sortedKVDirInfo.IsDir() { - return errors.Errorf("tikv-importer.sorted-kv-dir ('%s') is not a directory", storageSizeDir) - } - default: - return errors.Annotate(err, "invalid tikv-importer.sorted-kv-dir") - } - - if cfg.TikvImporter.DiskQuota == 0 { - enginesCount := uint64(cfg.App.IndexConcurrency + cfg.App.TableConcurrency) - writeAmount := uint64(cfg.App.RegionConcurrency) * uint64(cfg.Cron.CheckDiskQuota.Milliseconds()) - reservedSize := enginesCount*uint64(cfg.TikvImporter.EngineMemCacheSize) + writeAmount*autoDiskQuotaLocalReservedSpeed - - storageSize, err := common.GetStorageSize(storageSizeDir) - if err != nil { - return err - } - if storageSize.Available <= reservedSize { - return errors.Errorf( - "insufficient disk free space on `%s` (only %s, expecting >%s), please use a storage with enough free space, or specify `tikv-importer.disk-quota`", - cfg.TikvImporter.SortedKVDir, - units.BytesSize(float64(storageSize.Available)), - units.BytesSize(float64(reservedSize))) - } - cfg.TikvImporter.DiskQuota = ByteSize(storageSize.Available - reservedSize) + if err := cfg.CheckAndAdjustForLocalBackend(); err != nil { + return err } } @@ -634,25 +571,8 @@ func (cfg *Config) Adjust(ctx context.Context) error { return errors.Annotate(err, "invalid config: `mydumper.tidb.sql_mode` must be a valid SQL_MODE") } - if cfg.TiDB.Security == nil { - cfg.TiDB.Security = &cfg.Security - } - - switch cfg.TiDB.TLS { - case "": - if len(cfg.TiDB.Security.CAPath) > 0 { - cfg.TiDB.TLS = "cluster" - } else { - cfg.TiDB.TLS = "false" - } - case "cluster": - if len(cfg.Security.CAPath) == 0 { - return errors.New("invalid config: cannot set `tidb.tls` to 'cluster' without a [security] section") - } - case "false", "skip-verify", "preferred": - break - default: - return errors.Errorf("invalid config: unsupported `tidb.tls` config %s", cfg.TiDB.TLS) + if err := cfg.CheckAndAdjustSecurity(); err != nil { + return err } // mydumper.filter and black-white-list cannot co-exist. @@ -672,6 +592,92 @@ func (cfg *Config) Adjust(ctx context.Context) error { } } + if err := cfg.CheckAndAdjustTiDBPort(ctx, mustHaveInternalConnections); err != nil { + return err + } + cfg.AdjustMydumper() + cfg.AdjustCheckPoint() + return cfg.CheckAndAdjustFilePath() +} + +func (cfg *Config) CheckAndAdjustForLocalBackend() error { + if len(cfg.TikvImporter.SortedKVDir) == 0 { + return errors.Errorf("tikv-importer.sorted-kv-dir must not be empty!") + } + + storageSizeDir := filepath.Clean(cfg.TikvImporter.SortedKVDir) + sortedKVDirInfo, err := os.Stat(storageSizeDir) + switch { + case os.IsNotExist(err): + // the sorted-kv-dir does not exist, meaning we will create it automatically. + // so we extract the storage size from its parent directory. + storageSizeDir = filepath.Dir(storageSizeDir) + case err == nil: + if !sortedKVDirInfo.IsDir() { + return errors.Errorf("tikv-importer.sorted-kv-dir ('%s') is not a directory", storageSizeDir) + } + default: + return errors.Annotate(err, "invalid tikv-importer.sorted-kv-dir") + } + + if cfg.TikvImporter.DiskQuota == 0 { + enginesCount := uint64(cfg.App.IndexConcurrency + cfg.App.TableConcurrency) + writeAmount := uint64(cfg.App.RegionConcurrency) * uint64(cfg.Cron.CheckDiskQuota.Milliseconds()) + reservedSize := enginesCount*uint64(cfg.TikvImporter.EngineMemCacheSize) + writeAmount*autoDiskQuotaLocalReservedSpeed + + storageSize, err := common.GetStorageSize(storageSizeDir) + if err != nil { + return errors.Trace(err) + } + if storageSize.Available <= reservedSize { + return errors.Errorf( + "insufficient disk free space on `%s` (only %s, expecting >%s), please use a storage with enough free space, or specify `tikv-importer.disk-quota`", + cfg.TikvImporter.SortedKVDir, + units.BytesSize(float64(storageSize.Available)), + units.BytesSize(float64(reservedSize))) + } + cfg.TikvImporter.DiskQuota = ByteSize(storageSize.Available - reservedSize) + } + return nil +} + +func (cfg *Config) DefaultVarsForTiDBBackend() { + if cfg.App.IndexConcurrency == 0 { + cfg.App.IndexConcurrency = cfg.App.RegionConcurrency + } + if cfg.App.TableConcurrency == 0 { + cfg.App.TableConcurrency = cfg.App.RegionConcurrency + } +} + +func (cfg *Config) DefaultVarsForImporterAndLocalBackend() { + if cfg.App.IndexConcurrency == 0 { + cfg.App.IndexConcurrency = 2 + } + if cfg.App.TableConcurrency == 0 { + cfg.App.TableConcurrency = 6 + } + if cfg.TikvImporter.RangeConcurrency == 0 { + cfg.TikvImporter.RangeConcurrency = 16 + } + if cfg.TikvImporter.RegionSplitSize == 0 { + cfg.TikvImporter.RegionSplitSize = SplitRegionSize + } + if cfg.TiDB.DistSQLScanConcurrency == 0 { + cfg.TiDB.DistSQLScanConcurrency = defaultDistSQLScanConcurrency + } + if cfg.TiDB.BuildStatsConcurrency == 0 { + cfg.TiDB.BuildStatsConcurrency = defaultBuildStatsConcurrency + } + if cfg.TiDB.IndexSerialScanConcurrency == 0 { + cfg.TiDB.IndexSerialScanConcurrency = defaultIndexSerialScanConcurrency + } + if cfg.TiDB.ChecksumTableConcurrency == 0 { + cfg.TiDB.ChecksumTableConcurrency = defaultChecksumTableConcurrency + } +} + +func (cfg *Config) CheckAndAdjustTiDBPort(ctx context.Context, mustHaveInternalConnections bool) error { // automatically determine the TiDB port & PD address from TiDB settings if mustHaveInternalConnections && (cfg.TiDB.Port <= 0 || len(cfg.TiDB.PdAddr) == 0) { tls, err := cfg.ToTLS() @@ -699,47 +705,10 @@ func (cfg *Config) Adjust(ctx context.Context) error { if mustHaveInternalConnections && len(cfg.TiDB.PdAddr) == 0 { return errors.New("invalid `tidb.pd-addr` setting") } + return nil +} - // handle mydumper - if cfg.Mydumper.BatchSize <= 0 { - // if rows in source files are not sorted by primary key(if primary is number or cluster index enabled), - // the key range in each data engine may have overlap, thus a bigger engine size can somewhat alleviate it. - cfg.Mydumper.BatchSize = defaultBatchSize - } - if cfg.Mydumper.BatchImportRatio < 0.0 || cfg.Mydumper.BatchImportRatio >= 1.0 { - cfg.Mydumper.BatchImportRatio = 0.75 - } - if cfg.Mydumper.ReadBlockSize <= 0 { - cfg.Mydumper.ReadBlockSize = ReadBlockSize - } - if len(cfg.Mydumper.CharacterSet) == 0 { - cfg.Mydumper.CharacterSet = "auto" - } - - if len(cfg.Checkpoint.Schema) == 0 { - cfg.Checkpoint.Schema = "tidb_lightning_checkpoint" - } - if len(cfg.Checkpoint.Driver) == 0 { - cfg.Checkpoint.Driver = CheckpointDriverFile - } - if len(cfg.Checkpoint.DSN) == 0 { - switch cfg.Checkpoint.Driver { - case CheckpointDriverMySQL: - param := common.MySQLConnectParam{ - Host: cfg.TiDB.Host, - Port: cfg.TiDB.Port, - User: cfg.TiDB.User, - Password: cfg.TiDB.Psw, - SQLMode: mysql.DefaultSQLMode, - MaxAllowedPacket: defaultMaxAllowedPacket, - TLS: cfg.TiDB.TLS, - } - cfg.Checkpoint.DSN = param.ToDSN() - case CheckpointDriverFile: - cfg.Checkpoint.DSN = "/tmp/" + cfg.Checkpoint.Schema + ".pb" - } - } - +func (cfg *Config) CheckAndAdjustFilePath() error { var u *url.URL // An absolute Windows path like "C:\Users\XYZ" would be interpreted as @@ -751,6 +720,7 @@ func (cfg *Config) Adjust(ctx context.Context) error { // On Windows, the drive letter can only be single letters from "A:" to "Z:", // so this won't mistake "S3:" as a Windows path. if len(filepath.VolumeName(cfg.Mydumper.SourceDir)) == 0 { + var err error u, err = url.Parse(cfg.Mydumper.SourceDir) if err != nil { return errors.Trace(err) @@ -783,7 +753,73 @@ func (cfg *Config) Adjust(ctx context.Context) error { if !found { return errors.Errorf("Unsupported data-source-dir url '%s'", cfg.Mydumper.SourceDir) } + return nil +} + +func (cfg *Config) AdjustCheckPoint() { + if len(cfg.Checkpoint.Schema) == 0 { + cfg.Checkpoint.Schema = "tidb_lightning_checkpoint" + } + if len(cfg.Checkpoint.Driver) == 0 { + cfg.Checkpoint.Driver = CheckpointDriverFile + } + if len(cfg.Checkpoint.DSN) == 0 { + switch cfg.Checkpoint.Driver { + case CheckpointDriverMySQL: + param := common.MySQLConnectParam{ + Host: cfg.TiDB.Host, + Port: cfg.TiDB.Port, + User: cfg.TiDB.User, + Password: cfg.TiDB.Psw, + SQLMode: mysql.DefaultSQLMode, + MaxAllowedPacket: defaultMaxAllowedPacket, + TLS: cfg.TiDB.TLS, + } + cfg.Checkpoint.DSN = param.ToDSN() + case CheckpointDriverFile: + cfg.Checkpoint.DSN = "/tmp/" + cfg.Checkpoint.Schema + ".pb" + } + } +} +func (cfg *Config) AdjustMydumper() { + if cfg.Mydumper.BatchSize <= 0 { + // if rows in source files are not sorted by primary key(if primary is number or cluster index enabled), + // the key range in each data engine may have overlap, thus a bigger engine size can somewhat alleviate it. + cfg.Mydumper.BatchSize = defaultBatchSize + } + if cfg.Mydumper.BatchImportRatio < 0.0 || cfg.Mydumper.BatchImportRatio >= 1.0 { + cfg.Mydumper.BatchImportRatio = 0.75 + } + if cfg.Mydumper.ReadBlockSize <= 0 { + cfg.Mydumper.ReadBlockSize = ReadBlockSize + } + if len(cfg.Mydumper.CharacterSet) == 0 { + cfg.Mydumper.CharacterSet = "auto" + } +} + +func (cfg *Config) CheckAndAdjustSecurity() error { + if cfg.TiDB.Security == nil { + cfg.TiDB.Security = &cfg.Security + } + + switch cfg.TiDB.TLS { + case "": + if len(cfg.TiDB.Security.CAPath) > 0 { + cfg.TiDB.TLS = "cluster" + } else { + cfg.TiDB.TLS = "false" + } + case "cluster": + if len(cfg.Security.CAPath) == 0 { + return errors.New("invalid config: cannot set `tidb.tls` to 'cluster' without a [security] section") + } + case "false", "skip-verify", "preferred": + break + default: + return errors.Errorf("invalid config: unsupported `tidb.tls` config %s", cfg.TiDB.TLS) + } return nil } diff --git a/pkg/lightning/config/configlist.go b/pkg/lightning/config/configlist.go index c78e8903b..00ff6c79b 100644 --- a/pkg/lightning/config/configlist.go +++ b/pkg/lightning/config/configlist.go @@ -20,14 +20,14 @@ import ( "time" ) -// ConfigList is a goroutine-safe FIFO list of *Config, which supports removal +// List is a goroutine-safe FIFO list of *Config, which supports removal // from the middle. The list is not expected to be very long. -type ConfigList struct { +type List struct { cond *sync.Cond taskIDMap map[int64]*list.Element nodes list.List - // lastID records the largest task ID being Push()'ed to the ConfigList. + // lastID records the largest task ID being Push()'ed to the List. // In the rare case where two Push() are executed in the same nanosecond // (or the not-so-rare case where the clock's precision is lower than CPU // speed), we'll need to manually force one of the task to use the ID as @@ -36,8 +36,8 @@ type ConfigList struct { } // NewConfigList creates a new ConfigList instance. -func NewConfigList() *ConfigList { - return &ConfigList{ +func NewConfigList() *List { + return &List{ cond: sync.NewCond(new(sync.Mutex)), taskIDMap: make(map[int64]*list.Element), } @@ -45,7 +45,7 @@ func NewConfigList() *ConfigList { // Push adds a configuration to the end of the list. The field `cfg.TaskID` will // be modified to include a unique ID to identify this task. -func (cl *ConfigList) Push(cfg *Config) { +func (cl *List) Push(cfg *Config) { id := time.Now().UnixNano() cl.cond.L.Lock() defer cl.cond.L.Unlock() @@ -63,7 +63,7 @@ func (cl *ConfigList) Push(cfg *Config) { // input context expired. // // If the context expired, the error field will contain the error from context. -func (cl *ConfigList) Pop(ctx context.Context) (*Config, error) { +func (cl *List) Pop(ctx context.Context) (*Config, error) { res := make(chan *Config) go func() { @@ -91,7 +91,7 @@ func (cl *ConfigList) Pop(ctx context.Context) (*Config, error) { // Remove removes a task from the list given its task ID. Returns true if a task // is successfully removed, false if the task ID did not exist. -func (cl *ConfigList) Remove(taskID int64) bool { +func (cl *List) Remove(taskID int64) bool { cl.cond.L.Lock() defer cl.cond.L.Unlock() element, ok := cl.taskIDMap[taskID] @@ -105,7 +105,7 @@ func (cl *ConfigList) Remove(taskID int64) bool { // Get obtains a task from the list given its task ID. If the task ID did not // exist, the returned bool field will be false. -func (cl *ConfigList) Get(taskID int64) (*Config, bool) { +func (cl *List) Get(taskID int64) (*Config, bool) { cl.cond.L.Lock() defer cl.cond.L.Unlock() element, ok := cl.taskIDMap[taskID] @@ -116,7 +116,7 @@ func (cl *ConfigList) Get(taskID int64) (*Config, bool) { } // AllIDs returns a list of all task IDs in the list. -func (cl *ConfigList) AllIDs() []int64 { +func (cl *List) AllIDs() []int64 { cl.cond.L.Lock() defer cl.cond.L.Unlock() res := make([]int64, 0, len(cl.taskIDMap)) @@ -128,7 +128,7 @@ func (cl *ConfigList) AllIDs() []int64 { // MoveToFront moves a task to the front of the list. Returns true if the task // is successfully moved (including no-op), false if the task ID did not exist. -func (cl *ConfigList) MoveToFront(taskID int64) bool { +func (cl *List) MoveToFront(taskID int64) bool { cl.cond.L.Lock() defer cl.cond.L.Unlock() element, ok := cl.taskIDMap[taskID] @@ -141,7 +141,7 @@ func (cl *ConfigList) MoveToFront(taskID int64) bool { // MoveToBack moves a task to the back of the list. Returns true if the task is // successfully moved (including no-op), false if the task ID did not exist. -func (cl *ConfigList) MoveToBack(taskID int64) bool { +func (cl *List) MoveToBack(taskID int64) bool { cl.cond.L.Lock() defer cl.cond.L.Unlock() element, ok := cl.taskIDMap[taskID] diff --git a/pkg/lightning/glue/glue.go b/pkg/lightning/glue/glue.go index cf3a17f5d..1bb1b1c91 100644 --- a/pkg/lightning/glue/glue.go +++ b/pkg/lightning/glue/glue.go @@ -38,7 +38,7 @@ type Glue interface { GetParser() *parser.Parser GetTables(context.Context, string) ([]*model.TableInfo, error) GetSession(context.Context) (checkpoints.Session, error) - OpenCheckpointsDB(context.Context, *config.Config) (checkpoints.CheckpointsDB, error) + OpenCheckpointsDB(context.Context, *config.Config) (checkpoints.DB, error) // Record is used to report some information (key, value) to host TiDB, including progress, stage currently Record(string, uint64) } @@ -170,7 +170,7 @@ func (e *ExternalTiDBGlue) GetSession(ctx context.Context) (checkpoints.Session, return &sqlConnSession{conn: conn}, nil } -func (e *ExternalTiDBGlue) OpenCheckpointsDB(ctx context.Context, cfg *config.Config) (checkpoints.CheckpointsDB, error) { +func (e *ExternalTiDBGlue) OpenCheckpointsDB(ctx context.Context, cfg *config.Config) (checkpoints.DB, error) { return checkpoints.OpenCheckpointsDB(ctx, cfg) } diff --git a/pkg/lightning/lightning.go b/pkg/lightning/lightning.go index 143bf5ffb..a03bbfdb5 100755 --- a/pkg/lightning/lightning.go +++ b/pkg/lightning/lightning.go @@ -55,7 +55,7 @@ type Lightning struct { globalCfg *config.GlobalConfig globalTLS *common.TLS // taskCfgs is the list of task configurations enqueued in the server mode - taskCfgs *config.ConfigList + taskCfgs *config.List ctx context.Context shutdown context.CancelFunc // for whole lightning context server http.Server @@ -254,7 +254,9 @@ func (l *Lightning) run(taskCtx context.Context, taskCfg *config.Config, g glue. return } taskCfg.TiDB.Security.CAPath = "" - taskCfg.TiDB.Security.RegisterMySQL() + if err := taskCfg.TiDB.Security.RegisterMySQL(); err != nil { + log.L().Warn("failed to deregister TLS config", log.ShortError(err)) + } }() // initiation of default glue should be after RegisterMySQL, which is ready to be called after taskCfg.Adjust @@ -302,7 +304,7 @@ func (l *Lightning) run(taskCtx context.Context, taskCfg *config.Config, g glue. dbMetas := mdl.GetDatabases() web.BroadcastInitProgress(dbMetas) - var procedure *restore.RestoreController + var procedure *restore.Controller procedure, err = restore.NewRestoreController(ctx, dbMetas, taskCfg, s, g) if err != nil { log.L().Error("restore failed", log.ShortError(err)) @@ -336,7 +338,7 @@ func writeJSONError(w http.ResponseWriter, code int, prefix string, err error) { if err != nil { prefix += ": " + err.Error() } - json.NewEncoder(w).Encode(errorResponse{Error: prefix}) + _ = json.NewEncoder(w).Encode(errorResponse{Error: prefix}) } func parseTaskID(req *http.Request) (int64, string, error) { @@ -362,6 +364,11 @@ func (l *Lightning) handleTask(w http.ResponseWriter, req *http.Request) { switch req.Method { case http.MethodGet: taskID, _, err := parseTaskID(req) + // golint tells us to refactor this with switch stmt. + // However switch stmt doesn't support init-statements, + // hence if we follow it things might be worse. + // Anyway, this chain of if-else isn't unacceptable. + //nolint:gocritic if e, ok := err.(*strconv.NumError); ok && e.Num == "" { l.handleGetTask(w) } else if err == nil { @@ -401,7 +408,7 @@ func (l *Lightning) handleGetTask(w http.ResponseWriter) { l.cancelLock.Unlock() w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(response) + _ = json.NewEncoder(w).Encode(response) } func (l *Lightning) handleGetOneTask(w http.ResponseWriter, req *http.Request, taskID int64) { @@ -468,7 +475,7 @@ func (l *Lightning) handlePostTask(w http.ResponseWriter, req *http.Request) { l.taskCfgs.Push(cfg) w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(taskResponse{ID: cfg.TaskID}) + _ = json.NewEncoder(w).Encode(taskResponse{ID: cfg.TaskID}) } func (l *Lightning) handleDeleteOneTask(w http.ResponseWriter, req *http.Request) { @@ -501,7 +508,7 @@ func (l *Lightning) handleDeleteOneTask(w http.ResponseWriter, req *http.Request if cancelSuccess { w.WriteHeader(http.StatusOK) - w.Write([]byte("{}")) + _, _ = w.Write([]byte("{}")) } else { writeJSONError(w, http.StatusNotFound, "task ID not found", nil) } @@ -532,7 +539,7 @@ func (l *Lightning) handlePatchOneTask(w http.ResponseWriter, req *http.Request) if moveSuccess { w.WriteHeader(http.StatusOK) - w.Write([]byte("{}")) + _, _ = w.Write([]byte("{}")) } else { writeJSONError(w, http.StatusNotFound, "task ID not found", nil) } @@ -540,15 +547,15 @@ func (l *Lightning) handlePatchOneTask(w http.ResponseWriter, req *http.Request) func writeBytesCompressed(w http.ResponseWriter, req *http.Request, b []byte) { if !strings.Contains(req.Header.Get("Accept-Encoding"), "gzip") { - w.Write(b) + _, _ = w.Write(b) return } w.Header().Set("Content-Encoding", "gzip") w.WriteHeader(http.StatusOK) gw, _ := gzip.NewWriterLevel(w, gzip.BestSpeed) - gw.Write(b) - gw.Close() + _, _ = gw.Write(b) + _ = gw.Close() } func handleProgressTask(w http.ResponseWriter, req *http.Request) { @@ -558,7 +565,7 @@ func handleProgressTask(w http.ResponseWriter, req *http.Request) { writeBytesCompressed(w, req, res) } else { w.WriteHeader(http.StatusInternalServerError) - json.NewEncoder(w).Encode(err.Error()) + _ = json.NewEncoder(w).Encode(err.Error()) } } @@ -574,7 +581,7 @@ func handleProgressTable(w http.ResponseWriter, req *http.Request) { } else { w.WriteHeader(http.StatusInternalServerError) } - json.NewEncoder(w).Encode(err.Error()) + _ = json.NewEncoder(w).Encode(err.Error()) } } @@ -590,7 +597,7 @@ func handlePause(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) restore.DeliverPauser.Pause() log.L().Info("progress paused") - w.Write([]byte("{}")) + _, _ = w.Write([]byte("{}")) default: w.Header().Set("Allow", http.MethodGet+", "+http.MethodPut) @@ -606,7 +613,7 @@ func handleResume(w http.ResponseWriter, req *http.Request) { w.WriteHeader(http.StatusOK) restore.DeliverPauser.Resume() log.L().Info("progress resumed") - w.Write([]byte("{}")) + _, _ = w.Write([]byte("{}")) default: w.Header().Set("Allow", http.MethodPut) @@ -625,7 +632,7 @@ func handleLogLevel(w http.ResponseWriter, req *http.Request) { case http.MethodGet: logLevel.Level = log.Level() w.WriteHeader(http.StatusOK) - json.NewEncoder(w).Encode(logLevel) + _ = json.NewEncoder(w).Encode(logLevel) case http.MethodPut, http.MethodPost: if err := json.NewDecoder(req.Body).Decode(&logLevel); err != nil { @@ -636,7 +643,7 @@ func handleLogLevel(w http.ResponseWriter, req *http.Request) { log.L().Info("changed log level", zap.Stringer("old", oldLevel), zap.Stringer("new", logLevel.Level)) log.SetLevel(logLevel.Level) w.WriteHeader(http.StatusOK) - w.Write([]byte("{}")) + _, _ = w.Write([]byte("{}")) default: w.Header().Set("Allow", http.MethodGet+", "+http.MethodPut+", "+http.MethodPost) @@ -673,7 +680,7 @@ func checkSystemRequirement(cfg *config.Config, dbsMeta []*mydump.MDDatabaseMeta return nil } -/// checkSchemaConflict return error if checkpoint table scheme is conflict with data files +// checkSchemaConflict return error if checkpoint table scheme is conflict with data files func checkSchemaConflict(cfg *config.Config, dbsMeta []*mydump.MDDatabaseMeta) error { if cfg.Checkpoint.Enable && cfg.Checkpoint.Driver == config.CheckpointDriverMySQL { for _, db := range dbsMeta { diff --git a/pkg/lightning/lightning_test.go b/pkg/lightning/lightning_test.go index 910975afc..fcad54d3f 100644 --- a/pkg/lightning/lightning_test.go +++ b/pkg/lightning/lightning_test.go @@ -11,6 +11,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Contexts for HTTP requests communicating with a real HTTP server are essential, +// however, when the subject is a mocked server, it would probably be redundant. +//nolint:noctx package lightning import ( @@ -116,13 +119,13 @@ func (s *lightningServerSuite) SetUpTest(c *C) { s.lightning = New(cfg) s.taskCfgCh = make(chan *config.Config) s.lightning.ctx = context.WithValue(s.lightning.ctx, &taskCfgRecorderKey, s.taskCfgCh) - s.lightning.GoServe() + _ = s.lightning.GoServe() - failpoint.Enable("github.com/pingcap/br/pkg/lightning/SkipRunTask", "return") + _ = failpoint.Enable("github.com/pingcap/br/pkg/lightning/SkipRunTask", "return") } func (s *lightningServerSuite) TearDownTest(c *C) { - failpoint.Disable("github.com/pingcap/br/pkg/lightning/SkipRunTask") + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/SkipRunTask") s.lightning.Stop() } @@ -139,7 +142,9 @@ func (s *lightningServerSuite) TestRunServer(c *C) { c.Assert(data["error"], Equals, "server-mode not enabled") resp.Body.Close() - go s.lightning.RunServer() + go func() { + _ = s.lightning.RunServer() + }() time.Sleep(100 * time.Millisecond) req, err := http.NewRequest(http.MethodPut, url, nil) @@ -226,7 +231,9 @@ func (s *lightningServerSuite) TestGetDeleteTask(c *C) { return result.ID } - go s.lightning.RunServer() + go func() { + _ = s.lightning.RunServer() + }() time.Sleep(100 * time.Millisecond) // Check `GET /tasks` without any active tasks @@ -415,7 +422,7 @@ func (s *lightningServerSuite) TestHTTPAPIOutsideServerMode(c *C) { resp, err = http.DefaultClient.Do(req) c.Assert(err, IsNil) c.Assert(resp.StatusCode, Equals, http.StatusOK) - + c.Assert(resp.Body.Close(), IsNil) // ... and the task should be canceled now. c.Assert(<-errCh, Equals, context.Canceled) } @@ -473,7 +480,9 @@ func (s *lightningServerSuite) TestCheckSystemRequirement(c *C) { c.Assert(err, IsNil) err = failpoint.Enable("github.com/pingcap/br/pkg/lightning/backend/local/SetRlimitError", "return(true)") c.Assert(err, IsNil) - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/local/SetRlimitError") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/local/SetRlimitError") + }() // with this dbMetas, the estimated fds will be 2050, so should return error err = checkSystemRequirement(cfg, dbMetas) c.Assert(err, NotNil) @@ -483,7 +492,9 @@ func (s *lightningServerSuite) TestCheckSystemRequirement(c *C) { // the min rlimit should be bigger than the default min value (16384) err = failpoint.Enable("github.com/pingcap/br/pkg/lightning/backend/local/GetRlimitValue", "return(8200)") - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/local/GetRlimitValue") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/local/GetRlimitValue") + }() c.Assert(err, IsNil) err = checkSystemRequirement(cfg, dbMetas) c.Assert(err, IsNil) diff --git a/pkg/lightning/log/log.go b/pkg/lightning/log/log.go index 8ed79151f..50d938412 100644 --- a/pkg/lightning/log/log.go +++ b/pkg/lightning/log/log.go @@ -75,7 +75,9 @@ var ( // InitLogger initializes Lightning's and also the TiDB library's loggers. func InitLogger(cfg *Config, tidbLoglevel string) error { - logutil.InitLogger(&logutil.LogConfig{Config: pclog.Config{Level: tidbLoglevel}}) + if err := logutil.InitLogger(&logutil.LogConfig{Config: pclog.Config{Level: tidbLoglevel}}); err != nil { + return errors.Trace(err) + } logCfg := &pclog.Config{ Level: cfg.Level, diff --git a/pkg/lightning/metric/metric.go b/pkg/lightning/metric/metric.go index b4ffdc9c5..ef611828e 100644 --- a/pkg/lightning/metric/metric.go +++ b/pkg/lightning/metric/metric.go @@ -197,6 +197,7 @@ var ( ) ) +//nolint:gochecknoinits // TODO: refactor func init() { prometheus.MustRegister(IdleWorkersGauge) prometheus.MustRegister(ImporterEngineCounter) diff --git a/pkg/lightning/mydump/csv_parser.go b/pkg/lightning/mydump/csv_parser.go index 1e3db9dd3..7ff08f09f 100644 --- a/pkg/lightning/mydump/csv_parser.go +++ b/pkg/lightning/mydump/csv_parser.go @@ -17,7 +17,6 @@ import ( "bytes" "io" "strings" - "unicode" "github.com/pingcap/br/pkg/utils" @@ -37,8 +36,7 @@ var ( // CSVParser is basically a copy of encoding/csv, but special-cased for MySQL-like input. type CSVParser struct { blockParser - cfg *config.CSVConfig - escFlavor backslashEscapeFlavor + cfg *config.CSVConfig comma []byte quote []byte @@ -57,6 +55,7 @@ type CSVParser struct { lastRecord []string + escFlavor backslashEscapeFlavor // if set to true, csv parser will treat the first non-empty line as header line shouldParseHeader bool } @@ -131,6 +130,7 @@ func (parser *CSVParser) readByte() (byte, error) { return b, nil } +//nolint:unused // Maybe useful in the future. func (parser *CSVParser) readBytes(buf []byte) (int, error) { cnt := 0 for cnt < len(buf) { @@ -290,7 +290,7 @@ outside: continue } // skip lines only contain whitespaces - if err == nil && whitespaceLine && len(bytes.TrimFunc(parser.recordBuffer, unicode.IsSpace)) == 0 { + if err == nil && whitespaceLine && len(bytes.TrimSpace(parser.recordBuffer)) == 0 { parser.recordBuffer = parser.recordBuffer[:0] continue } @@ -391,9 +391,8 @@ func (parser *CSVParser) readQuotedField() error { if !bytes.Equal(b, parser.quote) { if parser.quote[0] == parser.comma[0] { return processComma() - } else { - return processDefault() } + return processDefault() } } parser.skipBytes(len(parser.quote)) @@ -567,10 +566,10 @@ func (parser *CSVParser) ReadColumns() error { return nil } -var newLineAsciiSet = makeByteSet([]byte{'\r', '\n'}) +var newLineASCIISet = makeByteSet([]byte{'\r', '\n'}) func indexOfNewLine(b []byte) int { - return IndexAnyByte(b, &newLineAsciiSet) + return IndexAnyByte(b, &newLineASCIISet) } func (parser *CSVParser) ReadUntilTokNewLine() (int64, error) { diff --git a/pkg/lightning/mydump/loader.go b/pkg/lightning/mydump/loader.go index 6bb0fcfb9..296427f80 100644 --- a/pkg/lightning/mydump/loader.go +++ b/pkg/lightning/mydump/loader.go @@ -91,11 +91,11 @@ type mdLoaderSetup struct { func NewMyDumpLoader(ctx context.Context, cfg *config.Config) (*MDLoader, error) { u, err := storage.ParseBackend(cfg.Mydumper.SourceDir, nil) if err != nil { - return nil, err + return nil, errors.Trace(err) } s, err := storage.New(ctx, u, &storage.ExternalStorageOptions{SkipCheckPath: true}) if err != nil { - return nil, err + return nil, errors.Trace(err) } return NewMyDumpLoaderWithStore(ctx, cfg, s) @@ -219,7 +219,7 @@ func (s *mdLoaderSetup) setup(ctx context.Context, store storage.ExternalStorage if !s.loader.noSchema { // setup database schema if len(s.dbSchemas) == 0 { - return errors.New("no schema create sql files found. Please either set `mydumper.no-schema` to true or add schema sql file for each database.") + return errors.New("no schema create sql files found. Please either set `mydumper.no-schema` to true or add schema sql file for each database") } for _, fileInfo := range s.dbSchemas { if _, dbExists := s.insertDB(fileInfo.TableName.Schema, fileInfo.FileMeta.Path); dbExists && s.loader.router == nil { @@ -267,12 +267,13 @@ func (s *mdLoaderSetup) setup(ctx context.Context, store storage.ExternalStorage for _, dbMeta := range s.loader.dbs { // Put the small table in the front of the slice which can avoid large table // take a long time to import and block small table to release index worker. - sort.SliceStable(dbMeta.Tables, func(i, j int) bool { - return dbMeta.Tables[i].TotalSize < dbMeta.Tables[j].TotalSize + meta := dbMeta + sort.SliceStable(meta.Tables, func(i, j int) bool { + return meta.Tables[i].TotalSize < meta.Tables[j].TotalSize }) // sort each table source files by sort-key - for _, tbMeta := range dbMeta.Tables { + for _, tbMeta := range meta.Tables { dataFiles := tbMeta.DataFiles sort.SliceStable(dataFiles, func(i, j int) bool { return dataFiles[i].FileMeta.SortKey < dataFiles[j].FileMeta.SortKey @@ -419,16 +420,15 @@ func (s *mdLoaderSetup) insertDB(dbName string, path string) (*MDDatabaseMeta, b dbIndex, ok := s.dbIndexMap[dbName] if ok { return s.loader.dbs[dbIndex], true - } else { - s.dbIndexMap[dbName] = len(s.loader.dbs) - ptr := &MDDatabaseMeta{ - Name: dbName, - SchemaFile: path, - charSet: s.loader.charSet, - } - s.loader.dbs = append(s.loader.dbs, ptr) - return ptr, false } + s.dbIndexMap[dbName] = len(s.loader.dbs) + ptr := &MDDatabaseMeta{ + Name: dbName, + SchemaFile: path, + charSet: s.loader.charSet, + } + s.loader.dbs = append(s.loader.dbs, ptr) + return ptr, false } func (s *mdLoaderSetup) insertTable(fileInfo FileInfo) (*MDTableMeta, bool, bool) { @@ -436,18 +436,17 @@ func (s *mdLoaderSetup) insertTable(fileInfo FileInfo) (*MDTableMeta, bool, bool tableIndex, ok := s.tableIndexMap[fileInfo.TableName] if ok { return dbMeta.Tables[tableIndex], dbExists, true - } else { - s.tableIndexMap[fileInfo.TableName] = len(dbMeta.Tables) - ptr := &MDTableMeta{ - DB: fileInfo.TableName.Schema, - Name: fileInfo.TableName.Name, - SchemaFile: fileInfo, - DataFiles: make([]FileInfo, 0, 16), - charSet: s.loader.charSet, - } - dbMeta.Tables = append(dbMeta.Tables, ptr) - return ptr, dbExists, false } + s.tableIndexMap[fileInfo.TableName] = len(dbMeta.Tables) + ptr := &MDTableMeta{ + DB: fileInfo.TableName.Schema, + Name: fileInfo.TableName.Name, + SchemaFile: fileInfo, + DataFiles: make([]FileInfo, 0, 16), + charSet: s.loader.charSet, + } + dbMeta.Tables = append(dbMeta.Tables, ptr) + return ptr, dbExists, false } func (s *mdLoaderSetup) insertView(fileInfo FileInfo) (bool, bool) { diff --git a/pkg/lightning/mydump/loader_test.go b/pkg/lightning/mydump/loader_test.go index 7b16bc074..9e77505a5 100644 --- a/pkg/lightning/mydump/loader_test.go +++ b/pkg/lightning/mydump/loader_test.go @@ -59,14 +59,13 @@ func (s *testMydumpLoaderSuite) SetUpTest(c *C) { s.cfg = newConfigWithSourceDir(s.sourceDir) } -func (s *testMydumpLoaderSuite) touch(c *C, filename ...string) string { +func (s *testMydumpLoaderSuite) touch(c *C, filename ...string) { components := make([]string, len(filename)+1) components = append(components, s.sourceDir) components = append(components, filename...) path := filepath.Join(components...) err := ioutil.WriteFile(path, nil, 0o644) c.Assert(err, IsNil) - return path } func (s *testMydumpLoaderSuite) mkdir(c *C, dirname string) { @@ -109,7 +108,7 @@ func (s *testMydumpLoaderSuite) TestLoader(c *C) { func (s *testMydumpLoaderSuite) TestEmptyDB(c *C) { _, err := md.NewMyDumpLoader(context.Background(), s.cfg) - c.Assert(err, ErrorMatches, "no schema create sql files found. Please either set `mydumper.no-schema` to true or add schema sql file for each database.") + c.Assert(err, ErrorMatches, "no schema create sql files found. Please either set `mydumper.no-schema` to true or add schema sql file for each database") } func (s *testMydumpLoaderSuite) TestDuplicatedDB(c *C) { @@ -522,7 +521,7 @@ func (s *testMydumpLoaderSuite) TestFileRouting(c *C) { s.touch(c, "d1/test2.001.sql") s.touch(c, "d1/v1-table.sql") s.touch(c, "d1/v1-view.sql") - _ = s.touch(c, "d1/t1-schema-create.sql") + s.touch(c, "d1/t1-schema-create.sql") s.touch(c, "d2/schema.sql") s.touch(c, "d2/abc-table.sql") s.touch(c, "abc.1.sql") diff --git a/pkg/lightning/mydump/parser.go b/pkg/lightning/mydump/parser.go index e08d3e3ac..18079199e 100644 --- a/pkg/lightning/mydump/parser.go +++ b/pkg/lightning/mydump/parser.go @@ -276,8 +276,8 @@ func unescape( ) string { if len(delim) > 0 { delim2 := delim + delim - if strings.Index(input, delim2) != -1 { - input = strings.Replace(input, delim2, delim, -1) + if strings.Contains(input, delim2) { + input = strings.ReplaceAll(input, delim2, delim) } } if escFlavor != backslashEscapeFlavorNone && strings.IndexByte(input, '\\') != -1 { @@ -500,13 +500,13 @@ func (parser *ChunkParser) ReadRow() error { case tokHexString: hexLit, err := types.ParseHexStr(string(content)) if err != nil { - return err + return errors.Trace(err) } value.SetBinaryLiteral(hexLit) case tokBinString: binLit, err := types.ParseBitStr(string(content)) if err != nil { - return err + return errors.Trace(err) } value.SetBinaryLiteral(binLit) default: @@ -527,6 +527,9 @@ func (parser *blockParser) LastRow() Row { // RecycleRow places the row object back into the allocation pool. func (parser *blockParser) RecycleRow(row Row) { + // We need farther benchmarking to make sure whether send a pointer + // (instead of a slice) here can improve performance. + //nolint:staticcheck parser.rowPool.Put(row.Row[:0]) } diff --git a/pkg/lightning/mydump/reader_test.go b/pkg/lightning/mydump/reader_test.go index 90d3f3967..fc94a56bb 100644 --- a/pkg/lightning/mydump/reader_test.go +++ b/pkg/lightning/mydump/reader_test.go @@ -27,8 +27,6 @@ import ( "github.com/pingcap/br/pkg/storage" ) -////////////////////////////////////////////////////////// - var _ = Suite(&testMydumpReaderSuite{}) type testMydumpReaderSuite struct{} @@ -59,12 +57,7 @@ func (s *testMydumpReaderSuite) TestExportStatementNoTrailingNewLine(c *C) { } func (s *testMydumpReaderSuite) TestExportStatementWithComment(c *C) { - dir := c.MkDir() - file, err := ioutil.TempFile(dir, "tidb_lightning_test_reader") - c.Assert(err, IsNil) - defer os.Remove(file.Name()) - - _, err = file.Write([]byte(` + s.exportStatmentShouldBe(c, ` /* whatever blabla multiple lines comment multiple lines comment @@ -73,29 +66,11 @@ func (s *testMydumpReaderSuite) TestExportStatementWithComment(c *C) { multiple lines comment */; CREATE DATABASE whatever; -`)) - c.Assert(err, IsNil) - stat, err := file.Stat() - c.Assert(err, IsNil) - err = file.Close() - c.Assert(err, IsNil) - - store, err := storage.NewLocalStorage(dir) - c.Assert(err, IsNil) - - f := FileInfo{FileMeta: SourceFileMeta{Path: stat.Name(), FileSize: stat.Size()}} - data, err := ExportStatement(context.TODO(), store, f, "auto") - c.Assert(err, IsNil) - c.Assert(data, DeepEquals, []byte("CREATE DATABASE whatever;")) +`, "CREATE DATABASE whatever;") } func (s *testMydumpReaderSuite) TestExportStatementWithCommentNoTrailingNewLine(c *C) { - dir := c.MkDir() - file, err := ioutil.TempFile(dir, "tidb_lightning_test_reader") - c.Assert(err, IsNil) - defer os.Remove(file.Name()) - - _, err = file.Write([]byte(` + s.exportStatmentShouldBe(c, ` /* whatever blabla multiple lines comment multiple lines comment @@ -103,7 +78,16 @@ func (s *testMydumpReaderSuite) TestExportStatementWithCommentNoTrailingNewLine( multiple lines comment multiple lines comment */; - CREATE DATABASE whatever;`)) + CREATE DATABASE whatever;`, "CREATE DATABASE whatever;") +} + +func (s *testMydumpReaderSuite) exportStatmentShouldBe(c *C, stmt string, expected string) { + dir := c.MkDir() + file, err := ioutil.TempFile(dir, "tidb_lightning_test_reader") + c.Assert(err, IsNil) + defer os.Remove(file.Name()) + + _, err = file.Write([]byte(stmt)) c.Assert(err, IsNil) stat, err := file.Stat() c.Assert(err, IsNil) @@ -115,7 +99,7 @@ func (s *testMydumpReaderSuite) TestExportStatementWithCommentNoTrailingNewLine( f := FileInfo{FileMeta: SourceFileMeta{Path: stat.Name(), FileSize: stat.Size()}} data, err := ExportStatement(context.TODO(), store, f, "auto") c.Assert(err, IsNil) - c.Assert(data, DeepEquals, []byte("CREATE DATABASE whatever;")) + c.Assert(data, DeepEquals, []byte(expected)) } func (s *testMydumpReaderSuite) TestExportStatementGBK(c *C) { @@ -169,11 +153,11 @@ func (s *testMydumpReaderSuite) TestExportStatementGibberishError(c *C) { type AlwaysErrorReadSeekCloser struct{} func (AlwaysErrorReadSeekCloser) Read([]byte) (int, error) { - return 0, errors.New("read error!") + return 0, errors.New("read error") } func (AlwaysErrorReadSeekCloser) Seek(int64, int) (int64, error) { - return 0, errors.New("seek error!") + return 0, errors.New("seek error") } func (AlwaysErrorReadSeekCloser) Close() error { @@ -193,5 +177,5 @@ func (s *testMydumpReaderSuite) TestExportStatementHandleNonEOFError(c *C) { f := FileInfo{FileMeta: SourceFileMeta{Path: "no-perm-file", FileSize: 1}} _, err := ExportStatement(ctx, mockStorage, f, "auto") - c.Assert(err, ErrorMatches, "read error!") + c.Assert(err, ErrorMatches, "read error") } diff --git a/pkg/lightning/mydump/region.go b/pkg/lightning/mydump/region.go index 9d7f1fb51..9b4634fba 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -58,8 +58,6 @@ func (reg *TableRegion) Size() int64 { return reg.Chunk.EndOffset - reg.Chunk.Offset } -//////////////////////////////////////////////////////////////// - func AllocateEngineIDs( filesRegions []*TableRegion, dataFileSizes []float64, @@ -262,7 +260,6 @@ func makeSourceFileRegion( // If a csv file is overlarge, we need to split it into multiple regions. // Note: We can only split a csv file whose format is strict. if isCsvFile && dataFileSize > int64(cfg.Mydumper.MaxRegionSize) && cfg.Mydumper.StrictFormat { - _, regions, subFileSizes, err := SplitLargeFile(ctx, meta, cfg, fi, divisor, 0, ioWorkers, store) return regions, subFileSizes, err } @@ -302,6 +299,9 @@ func makeParquetFileRegion( return prevRowIdxMax, nil, errors.Trace(err) } numberRows, err := ReadParquetFileRowCount(ctx, store, r, dataFile.FileMeta.Path) + if err != nil { + return 0, nil, errors.Trace(err) + } rowIDMax := prevRowIdxMax + numberRows region := &TableRegion{ DB: meta.DB, @@ -333,7 +333,7 @@ func SplitLargeFile( prevRowIdxMax int64, ioWorker *worker.Pool, store storage.ExternalStorage, -) (prevRowIdMax int64, regions []*TableRegion, dataFileSizes []float64, err error) { +) (prevRowIDMax int64, regions []*TableRegion, dataFileSizes []float64, err error) { maxRegionSize := int64(cfg.Mydumper.MaxRegionSize) dataFileSizes = make([]float64, 0, dataFile.FileMeta.FileSize/maxRegionSize+1) startOffset, endOffset := int64(0), maxRegionSize @@ -360,7 +360,7 @@ func SplitLargeFile( return 0, nil, nil, err } parser := NewCSVParser(&cfg.Mydumper.CSV, r, int64(cfg.Mydumper.ReadBlockSize), ioWorker, false) - if err = parser.SetPos(endOffset, prevRowIdMax); err != nil { + if err = parser.SetPos(endOffset, prevRowIDMax); err != nil { return 0, nil, nil, err } pos, err := parser.ReadUntilTokNewLine() diff --git a/pkg/lightning/mydump/region_test.go b/pkg/lightning/mydump/region_test.go index 4c0062721..e44915ce1 100644 --- a/pkg/lightning/mydump/region_test.go +++ b/pkg/lightning/mydump/region_test.go @@ -41,21 +41,6 @@ func (s *testMydumpRegionSuite) TearDownSuite(c *C) {} // "tbl_multi_index": 10000, // } -func getFileSize(file string) (int64, error) { - fd, err := os.Open(file) - if err != nil { - return -1, err - } - defer fd.Close() - - fstat, err := fd.Stat() - if err != nil { - return -1, err - } - - return fstat.Size(), nil -} - /* TODO : test with specified 'regionBlockSize' ... */ diff --git a/pkg/lightning/mydump/router.go b/pkg/lightning/mydump/router.go index 10b87e0b1..8ec5857a3 100644 --- a/pkg/lightning/mydump/router.go +++ b/pkg/lightning/mydump/router.go @@ -196,7 +196,6 @@ func (p regexRouterParser) Parse(r *config.FileRouteRule) (*RegexRouter, error) r.Type = quoteTmplFn(r.Type) r.Compression = quoteTmplFn(r.Compression) r.Key = quoteTmplFn(r.Key) - } pattern, err := regexp.Compile(r.Pattern) if err != nil { diff --git a/pkg/lightning/mydump/router_test.go b/pkg/lightning/mydump/router_test.go index e308bd842..8c5e2f178 100644 --- a/pkg/lightning/mydump/router_test.go +++ b/pkg/lightning/mydump/router_test.go @@ -145,7 +145,7 @@ func (t *testFileRouterSuite) TestMultiRouteRule(c *C) { "/test/123/my_schema.my_table.sql": {"my_schema", "my_table", "", "", "sql"}, "my_dir/my_schema.my_table.csv": {"my_schema", "my_table", "", "", "csv"}, "my_schema.my_table.0001.sql": {"my_schema", "my_table", "0001", "", "sql"}, - //"my_schema.my_table.0001.sql.gz": {"my_schema", "my_table", "0001", "gz", "sql"}, + // "my_schema.my_table.0001.sql.gz": {"my_schema", "my_table", "0001", "gz", "sql"}, } for path, fields := range inputOutputMap { res, err := r.Route(path) diff --git a/pkg/lightning/restore/checksum.go b/pkg/lightning/restore/checksum.go index 53395896b..0614ad2bc 100644 --- a/pkg/lightning/restore/checksum.go +++ b/pkg/lightning/restore/checksum.go @@ -21,7 +21,7 @@ import ( "go.uber.org/zap" "github.com/pingcap/br/pkg/checksum" - . "github.com/pingcap/br/pkg/lightning/checkpoints" + "github.com/pingcap/br/pkg/lightning/checkpoints" "github.com/pingcap/br/pkg/lightning/common" "github.com/pingcap/br/pkg/lightning/config" "github.com/pingcap/br/pkg/lightning/log" @@ -52,10 +52,10 @@ type RemoteChecksum struct { } type ChecksumManager interface { - Checksum(ctx context.Context, tableInfo *TidbTableInfo) (*RemoteChecksum, error) + Checksum(ctx context.Context, tableInfo *checkpoints.TidbTableInfo) (*RemoteChecksum, error) } -func newChecksumManager(ctx context.Context, rc *RestoreController) (ChecksumManager, error) { +func newChecksumManager(ctx context.Context, rc *Controller) (ChecksumManager, error) { // if we don't need checksum, just return nil if rc.cfg.TikvImporter.Backend == config.BackendTiDB || rc.cfg.PostRestore.Checksum == config.OpLevelOff { return nil, nil @@ -114,7 +114,7 @@ func newTiDBChecksumExecutor(db *sql.DB) *tidbChecksumExecutor { } } -func (e *tidbChecksumExecutor) Checksum(ctx context.Context, tableInfo *TidbTableInfo) (*RemoteChecksum, error) { +func (e *tidbChecksumExecutor) Checksum(ctx context.Context, tableInfo *checkpoints.TidbTableInfo) (*RemoteChecksum, error) { var err error if err = e.manager.addOneJob(ctx, e.db); err != nil { return nil, err @@ -149,7 +149,7 @@ func (e *tidbChecksumExecutor) Checksum(ctx context.Context, tableInfo *TidbTabl // DoChecksum do checksum for tables. // table should be in ., format. e.g. foo.bar -func DoChecksum(ctx context.Context, table *TidbTableInfo) (*RemoteChecksum, error) { +func DoChecksum(ctx context.Context, table *checkpoints.TidbTableInfo) (*RemoteChecksum, error) { var err error manager, ok := ctx.Value(&checksumManagerKey).(ChecksumManager) if !ok { @@ -195,7 +195,7 @@ func (m *gcLifeTimeManager) addOneJob(ctx context.Context, db *sql.DB) error { return err } } - m.runningJobs += 1 + m.runningJobs++ return nil } @@ -207,7 +207,7 @@ func (m *gcLifeTimeManager) removeOneJob(ctx context.Context, db *sql.DB) { m.runningJobsLock.Lock() defer m.runningJobsLock.Unlock() - m.runningJobs -= 1 + m.runningJobs-- if m.runningJobs == 0 { err := UpdateGCLifeTime(ctx, db, m.oriGCLifeTime) if err != nil { @@ -266,7 +266,7 @@ func newTiKVChecksumManager(client kv.Client, pdClient pd.Client, distSQLScanCon } } -func (e *tikvChecksumManager) checksumDB(ctx context.Context, tableInfo *TidbTableInfo) (*RemoteChecksum, error) { +func (e *tikvChecksumManager) checksumDB(ctx context.Context, tableInfo *checkpoints.TidbTableInfo) (*RemoteChecksum, error) { physicalTS, logicalTS, err := e.manager.pdClient.GetTS(ctx) if err != nil { return nil, errors.Annotate(err, "fetch tso from pd failed") @@ -312,7 +312,7 @@ func (e *tikvChecksumManager) checksumDB(ctx context.Context, tableInfo *TidbTab return nil, err } -func (e *tikvChecksumManager) Checksum(ctx context.Context, tableInfo *TidbTableInfo) (*RemoteChecksum, error) { +func (e *tikvChecksumManager) Checksum(ctx context.Context, tableInfo *checkpoints.TidbTableInfo) (*RemoteChecksum, error) { tbl := common.UniqueTable(tableInfo.DB, tableInfo.Name) err := e.manager.addOneJob(ctx, tbl, oracle.ComposeTS(time.Now().Unix()*1000, 0)) if err != nil { @@ -356,7 +356,7 @@ type gcTTLManager struct { pdClient pd.Client // tableGCSafeTS is a binary heap that stored active checksum jobs GC safe point ts tableGCSafeTS []*tableChecksumTS - currentTs uint64 + currentTS uint64 serviceID string // 0 for not start, otherwise started started uint32 @@ -376,15 +376,15 @@ func (m *gcTTLManager) addOneJob(ctx context.Context, table string, ts uint64) e } m.lock.Lock() defer m.lock.Unlock() - var curTs uint64 + var curTS uint64 if len(m.tableGCSafeTS) > 0 { - curTs = m.tableGCSafeTS[0].gcSafeTS + curTS = m.tableGCSafeTS[0].gcSafeTS } m.Push(&tableChecksumTS{table: table, gcSafeTS: ts}) heap.Fix(m, len(m.tableGCSafeTS)-1) - m.currentTs = m.tableGCSafeTS[0].gcSafeTS - if curTs == 0 || m.currentTs < curTs { - return m.doUpdateGCTTL(ctx, m.currentTs) + m.currentTS = m.tableGCSafeTS[0].gcSafeTS + if curTS == 0 || m.currentTS < curTS { + return m.doUpdateGCTTL(ctx, m.currentTS) } return nil } @@ -409,18 +409,18 @@ func (m *gcTTLManager) removeOneJob(table string) { } } - var newTs uint64 + var newTS uint64 if len(m.tableGCSafeTS) > 0 { - newTs = m.tableGCSafeTS[0].gcSafeTS + newTS = m.tableGCSafeTS[0].gcSafeTS } - m.currentTs = newTs + m.currentTS = newTS } func (m *gcTTLManager) updateGCTTL(ctx context.Context) error { m.lock.Lock() - currentTs := m.currentTs + currentTS := m.currentTS m.lock.Unlock() - return m.doUpdateGCTTL(ctx, currentTs) + return m.doUpdateGCTTL(ctx, currentTS) } func (m *gcTTLManager) doUpdateGCTTL(ctx context.Context, ts uint64) error { diff --git a/pkg/lightning/restore/checksum_test.go b/pkg/lightning/restore/checksum_test.go index 25c73caa9..2ca86f537 100644 --- a/pkg/lightning/restore/checksum_test.go +++ b/pkg/lightning/restore/checksum_test.go @@ -176,7 +176,7 @@ func (s *checksumSuite) TestDoChecksumWithTikv(c *C) { kvClient.maxErrCount = i kvClient.curErrCount = 0 checksumExec := &tikvChecksumManager{manager: newGCTTLManager(pdClient), client: kvClient} - startTs := oracle.ComposeTS(time.Now().Unix()*1000, 0) + startTS := oracle.ComposeTS(time.Now().Unix()*1000, 0) ctx := context.WithValue(context.Background(), &checksumManagerKey, checksumExec) _, err = DoChecksum(ctx, &TidbTableInfo{DB: "test", Name: "t", Core: tableInfo}) // with max error retry < maxErrorRetryCount, the checksum can success @@ -190,7 +190,7 @@ func (s *checksumSuite) TestDoChecksumWithTikv(c *C) { // after checksum, safepint should be small than start ts ts := pdClient.currentSafePoint() // 1ms for the schedule deviation - c.Assert(ts <= startTs+1, IsTrue) + c.Assert(ts <= startTS+1, IsTrue) c.Assert(atomic.LoadUint32(&checksumExec.manager.started) > 0, IsTrue) } } @@ -311,23 +311,23 @@ func (s *checksumSuite) TestGcTTLManagerMulti(c *C) { for i := uint64(1); i <= 5; i++ { err := manager.addOneJob(ctx, fmt.Sprintf("test%d", i), i) c.Assert(err, IsNil) - c.Assert(manager.currentTs, Equals, uint64(1)) + c.Assert(manager.currentTS, Equals, uint64(1)) } manager.removeOneJob("test2") - c.Assert(manager.currentTs, Equals, uint64(1)) + c.Assert(manager.currentTS, Equals, uint64(1)) manager.removeOneJob("test1") - c.Assert(manager.currentTs, Equals, uint64(3)) + c.Assert(manager.currentTS, Equals, uint64(3)) manager.removeOneJob("test3") - c.Assert(manager.currentTs, Equals, uint64(4)) + c.Assert(manager.currentTS, Equals, uint64(4)) manager.removeOneJob("test4") - c.Assert(manager.currentTs, Equals, uint64(5)) + c.Assert(manager.currentTS, Equals, uint64(5)) manager.removeOneJob("test5") - c.Assert(manager.currentTs, Equals, uint64(0)) + c.Assert(manager.currentTS, Equals, uint64(0)) } func (s *checksumSuite) TestPdServiceID(c *C) { diff --git a/pkg/lightning/restore/const.go b/pkg/lightning/restore/const.go deleted file mode 100644 index 082940c7e..000000000 --- a/pkg/lightning/restore/const.go +++ /dev/null @@ -1,18 +0,0 @@ -// 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 restore - -const ( - defReadBlockSize int64 = 1024 * 128 // TODO ... config -) diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 88ce5816c..42fbcf081 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -45,7 +45,7 @@ import ( "github.com/pingcap/br/pkg/lightning/backend/kv" "github.com/pingcap/br/pkg/lightning/backend/local" "github.com/pingcap/br/pkg/lightning/backend/tidb" - . "github.com/pingcap/br/pkg/lightning/checkpoints" + "github.com/pingcap/br/pkg/lightning/checkpoints" "github.com/pingcap/br/pkg/lightning/common" "github.com/pingcap/br/pkg/lightning/config" "github.com/pingcap/br/pkg/lightning/glue" @@ -83,8 +83,8 @@ const ( // DeliverPauser is a shared pauser to pause progress to (*chunkRestore).encodeLoop var DeliverPauser = common.NewPauser() +// nolint:gochecknoinits // TODO: refactor func init() { - // used in integration tests failpoint.Inject("SetMinDeliverBytes", func(v failpoint.Value) { minDeliverBytes = uint64(v.(int)) }) @@ -92,11 +92,11 @@ func init() { type saveCp struct { tableName string - merger TableCheckpointMerger + merger checkpoints.TableCheckpointMerger } type errorSummary struct { - status CheckpointStatus + status checkpoints.CheckpointStatus err error } @@ -131,7 +131,7 @@ func (es *errorSummaries) emitLog() { } } -func (es *errorSummaries) record(tableName string, err error, status CheckpointStatus) { +func (es *errorSummaries) record(tableName string, err error, status checkpoints.CheckpointStatus) { es.Lock() defer es.Unlock() es.summary[tableName] = errorSummary{status: status, err: err} @@ -143,34 +143,35 @@ const ( diskQuotaStateImporting ) -type RestoreController struct { - cfg *config.Config - dbMetas []*mydump.MDDatabaseMeta - dbInfos map[string]*TidbDBInfo - tableWorkers *worker.Pool - indexWorkers *worker.Pool - regionWorkers *worker.Pool - ioWorkers *worker.Pool - checksumWorks *worker.Pool - pauser *common.Pauser - backend backend.Backend - tidbGlue glue.Glue +type Controller struct { + cfg *config.Config + dbMetas []*mydump.MDDatabaseMeta + dbInfos map[string]*checkpoints.TidbDBInfo + tableWorkers *worker.Pool + indexWorkers *worker.Pool + regionWorkers *worker.Pool + ioWorkers *worker.Pool + checksumWorks *worker.Pool + pauser *common.Pauser + backend backend.Backend + tidbGlue glue.Glue + alterTableLock sync.Mutex - compactState int32 sysVars map[string]string tls *common.TLS + errorSummaries errorSummaries - checkpointsDB CheckpointsDB + checkpointsDB checkpoints.DB saveCpCh chan saveCp checkpointsWg sync.WaitGroup closedEngineLimit *worker.Pool store storage.ExternalStorage - checksumManager ChecksumManager diskQuotaLock sync.RWMutex diskQuotaState int32 + compactState int32 // commit ts for local and importer backend ts uint64 @@ -182,7 +183,7 @@ func NewRestoreController( cfg *config.Config, s storage.ExternalStorage, g glue.Glue, -) (*RestoreController, error) { +) (*Controller, error) { return NewRestoreControllerWithPauser(ctx, dbMetas, cfg, s, DeliverPauser, g) } @@ -193,7 +194,7 @@ func NewRestoreControllerWithPauser( s storage.ExternalStorage, pauser *common.Pauser, g glue.Glue, -) (*RestoreController, error) { +) (*Controller, error) { tls, err := cfg.ToTLS() if err != nil { return nil, err @@ -266,7 +267,7 @@ func NewRestoreControllerWithPauser( ts = oracle.ComposeTS(physical, logical) } - rc := &RestoreController{ + rc := &Controller{ cfg: cfg, dbMetas: dbMetas, tableWorkers: worker.NewPool(ctx, cfg.App.TableConcurrency, "table"), @@ -292,12 +293,12 @@ func NewRestoreControllerWithPauser( return rc, nil } -func (rc *RestoreController) Close() { +func (rc *Controller) Close() { rc.backend.Close() rc.tidbGlue.GetSQLExecutor().Close() } -func (rc *RestoreController) Run(ctx context.Context) error { +func (rc *Controller) Run(ctx context.Context) error { opts := []func(context.Context) error{ rc.checkRequirements, rc.setGlobalVariables, @@ -483,7 +484,7 @@ func (worker *restoreSchemaWorker) makeJobs(dbMetas []*mydump.MDDatabaseMeta) er } func (worker *restoreSchemaWorker) doJob() { - var session Session + var session checkpoints.Session defer func() { if session != nil { session.Close() @@ -581,7 +582,7 @@ func (worker *restoreSchemaWorker) appendJob(job *schemaJob) error { } } -func (rc *RestoreController) checkTableEmpty(ctx context.Context, tableName string) error { +func (rc *Controller) checkTableEmpty(ctx context.Context, tableName string) error { db, err := rc.tidbGlue.GetDB() if err != nil { return err @@ -601,7 +602,7 @@ func (rc *RestoreController) checkTableEmpty(ctx context.Context, tableName stri } } -func (rc *RestoreController) restoreSchema(ctx context.Context) error { +func (rc *Controller) restoreSchema(ctx context.Context) error { if !rc.cfg.Mydumper.NoSchema { logTask := log.L().Begin(zap.InfoLevel, "restore all schema") concurrency := utils.MinInt(rc.cfg.App.RegionConcurrency, 8) @@ -668,6 +669,31 @@ func (rc *RestoreController) restoreSchema(ctx context.Context) error { os.Exit(0) }) + if rc.cfg.TikvImporter.Backend != config.BackendTiDB { + for _, dbMeta := range rc.dbMetas { + for _, tableMeta := range dbMeta.Tables { + tableName := common.UniqueTable(dbMeta.Name, tableMeta.Name) + + // if checkpoint enable and not missing, we skip the check table empty progress. + if rc.cfg.Checkpoint.Enable { + dbCp, err := rc.checkpointsDB.Get(ctx, tableName) + if err != nil { + return errors.Trace(err) + } + + if dbCp.Status > checkpoints.CheckpointStatusMissing { + continue + } + } + + err := rc.checkTableEmpty(ctx, tableName) + if err != nil { + return err + } + } + } + } + go rc.listenCheckpointUpdates() rc.sysVars = ObtainImportantVariables(ctx, rc.tidbGlue.GetSQLExecutor()) @@ -678,7 +704,7 @@ func (rc *RestoreController) restoreSchema(ctx context.Context) error { } // verifyCheckpoint check whether previous task checkpoint is compatible with task config -func verifyCheckpoint(cfg *config.Config, taskCp *TaskCheckpoint) error { +func verifyCheckpoint(cfg *config.Config, taskCp *checkpoints.TaskCheckpoint) error { if taskCp == nil { return nil } @@ -734,7 +760,7 @@ func verifyCheckpoint(cfg *config.Config, taskCp *TaskCheckpoint) error { } // for local backend, we should check if local SST exists in disk, otherwise we'll lost data -func verifyLocalFile(ctx context.Context, cpdb CheckpointsDB, dir string) error { +func verifyLocalFile(ctx context.Context, cpdb checkpoints.DB, dir string) error { targetTables, err := cpdb.GetLocalStoringTables(ctx) if err != nil { return errors.Trace(err) @@ -742,7 +768,7 @@ func verifyLocalFile(ctx context.Context, cpdb CheckpointsDB, dir string) error for tableName, engineIDs := range targetTables { for _, engineID := range engineIDs { _, eID := backend.MakeUUID(tableName, engineID) - file := local.File{Uuid: eID} + file := local.File{UUID: eID} err := file.Exist(dir) if err != nil { log.L().Error("can't find local file", @@ -755,7 +781,7 @@ func verifyLocalFile(ctx context.Context, cpdb CheckpointsDB, dir string) error return nil } -func (rc *RestoreController) estimateChunkCountIntoMetrics(ctx context.Context) error { +func (rc *Controller) estimateChunkCountIntoMetrics(ctx context.Context) error { estimatedChunkCount := 0.0 estimatedEngineCnt := int64(0) batchSize := int64(rc.cfg.Mydumper.BatchSize) @@ -768,11 +794,11 @@ func (rc *RestoreController) estimateChunkCountIntoMetrics(ctx context.Context) } fileChunks := make(map[string]float64) - for engineId, eCp := range dbCp.Engines { - if eCp.Status < CheckpointStatusImported { + for engineID, eCp := range dbCp.Engines { + if eCp.Status < checkpoints.CheckpointStatusImported { estimatedEngineCnt++ } - if engineId == indexEngineID { + if engineID == indexEngineID { continue } for _, c := range eCp.Chunks { @@ -797,10 +823,10 @@ func (rc *RestoreController) estimateChunkCountIntoMetrics(ctx context.Context) if fileMeta.FileMeta.FileSize > int64(cfg.MaxRegionSize) && cfg.StrictFormat && !cfg.CSV.Header { estimatedChunkCount += math.Round(float64(fileMeta.FileMeta.FileSize) / float64(cfg.MaxRegionSize)) } else { - estimatedChunkCount += 1 + estimatedChunkCount++ } } else { - estimatedChunkCount += 1 + estimatedChunkCount++ } } } @@ -812,8 +838,8 @@ func (rc *RestoreController) estimateChunkCountIntoMetrics(ctx context.Context) return nil } -func (rc *RestoreController) saveStatusCheckpoint(tableName string, engineID int32, err error, statusIfSucceed CheckpointStatus) { - merger := &StatusCheckpointMerger{Status: statusIfSucceed, EngineID: engineID} +func (rc *Controller) saveStatusCheckpoint(tableName string, engineID int32, err error, statusIfSucceed checkpoints.CheckpointStatus) { + merger := &checkpoints.StatusCheckpointMerger{Status: statusIfSucceed, EngineID: engineID} log.L().Debug("update checkpoint", zap.String("table", tableName), zap.Int32("engine_id", engineID), zap.Uint8("new_status", uint8(statusIfSucceed)), zap.Error(err)) @@ -828,7 +854,7 @@ func (rc *RestoreController) saveStatusCheckpoint(tableName string, engineID int return } - if engineID == WholeTableEngineID { + if engineID == checkpoints.WholeTableEngineID { metric.RecordTableCount(statusIfSucceed.MetricName(), err) } else { metric.RecordEngineCount(statusIfSucceed.MetricName(), err) @@ -838,11 +864,11 @@ func (rc *RestoreController) saveStatusCheckpoint(tableName string, engineID int } // listenCheckpointUpdates will combine several checkpoints together to reduce database load. -func (rc *RestoreController) listenCheckpointUpdates() { +func (rc *Controller) listenCheckpointUpdates() { rc.checkpointsWg.Add(1) var lock sync.Mutex - coalesed := make(map[string]*TableCheckpointDiff) + coalesed := make(map[string]*checkpoints.TableCheckpointDiff) hasCheckpoint := make(chan struct{}, 1) defer close(hasCheckpoint) @@ -851,7 +877,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { for range hasCheckpoint { lock.Lock() cpd := coalesed - coalesed = make(map[string]*TableCheckpointDiff) + coalesed = make(map[string]*checkpoints.TableCheckpointDiff) lock.Unlock() if len(cpd) > 0 { @@ -866,7 +892,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { lock.Lock() cpd, ok := coalesed[scp.tableName] if !ok { - cpd = NewTableCheckpointDiff() + cpd = checkpoints.NewTableCheckpointDiff() coalesed[scp.tableName] = cpd } scp.merger.MergeInto(cpd) @@ -878,42 +904,48 @@ func (rc *RestoreController) listenCheckpointUpdates() { lock.Unlock() + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfImportedChunk", func(val failpoint.Value) { - if merger, ok := scp.merger.(*ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { + if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { rc.checkpointsWg.Done() rc.checkpointsWg.Wait() panic("forcing failure due to FailIfImportedChunk") } }) + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfStatusBecomes", func(val failpoint.Value) { - if merger, ok := scp.merger.(*StatusCheckpointMerger); ok && merger.EngineID >= 0 && int(merger.Status) == val.(int) { + if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && merger.EngineID >= 0 && int(merger.Status) == val.(int) { rc.checkpointsWg.Done() rc.checkpointsWg.Wait() panic("forcing failure due to FailIfStatusBecomes") } }) + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfIndexEngineImported", func(val failpoint.Value) { - if merger, ok := scp.merger.(*StatusCheckpointMerger); ok && - merger.EngineID == WholeTableEngineID && - merger.Status == CheckpointStatusIndexImported && val.(int) > 0 { + if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && + merger.EngineID == checkpoints.WholeTableEngineID && + merger.Status == checkpoints.CheckpointStatusIndexImported && val.(int) > 0 { rc.checkpointsWg.Done() rc.checkpointsWg.Wait() panic("forcing failure due to FailIfIndexEngineImported") } }) + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("KillIfImportedChunk", func(val failpoint.Value) { - if merger, ok := scp.merger.(*ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { - common.KillMySelf() + if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { + if err := common.KillMySelf(); err != nil { + log.L().Warn("KillMySelf() failed to kill itself", log.ShortError(err)) + } } }) } rc.checkpointsWg.Done() } -func (rc *RestoreController) runPeriodicActions(ctx context.Context, stop <-chan struct{}) { +func (rc *Controller) runPeriodicActions(ctx context.Context, stop <-chan struct{}) { // a nil channel blocks forever. // if the cron duration is zero we use the nil channel to skip the action. var logProgressChan <-chan time.Time @@ -983,16 +1015,16 @@ func (rc *RestoreController) runPeriodicActions(ctx context.Context, stop <-chan var state string var remaining zap.Field - if finished >= estimated { + switch { + case finished >= estimated: if engineFinished < engineEstimated { state = "importing" } else { state = "post-processing" } - remaining = zap.Skip() - } else if finished > 0 { + case finished > 0: state = "writing" - } else { + default: state = "preparing" } @@ -1055,7 +1087,7 @@ func (rc *RestoreController) runPeriodicActions(ctx context.Context, stop <-chan var checksumManagerKey struct{} -func (rc *RestoreController) restoreTables(ctx context.Context) error { +func (rc *Controller) restoreTables(ctx context.Context) error { logTask := log.L().Begin(zap.InfoLevel, "restore all tables data") // for local backend, we should disable some pd scheduler and change some settings, to @@ -1086,7 +1118,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { type task struct { tr *TableRestore - cp *TableCheckpoint + cp *checkpoints.TableCheckpoint } totalTables := 0 @@ -1130,7 +1162,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { } // first collect all tables where the checkpoint is invalid - allInvalidCheckpoints := make(map[string]CheckpointStatus) + allInvalidCheckpoints := make(map[string]checkpoints.CheckpointStatus) // collect all tables whose checkpoint's tableID can't match current tableID allDirtyCheckpoints := make(map[string]struct{}) for _, dbMeta := range rc.dbMetas { @@ -1149,7 +1181,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { if err != nil { return errors.Trace(err) } - if cp.Status <= CheckpointStatusMaxInvalid { + if cp.Status <= checkpoints.CheckpointStatusMaxInvalid { allInvalidCheckpoints[tableName] = cp.Status } else if cp.TableID > 0 && cp.TableID != tableInfo.ID { allDirtyCheckpoints[tableName] = struct{}{} @@ -1169,7 +1201,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { var action strings.Builder action.WriteString("./tidb-lightning-ctl --checkpoint-error-") switch failedStep { - case CheckpointStatusAlteredAutoInc, CheckpointStatusAnalyzed: + case checkpoints.CheckpointStatusAlteredAutoInc, checkpoints.CheckpointStatusAnalyzed: action.WriteString("ignore") default: action.WriteString("destroy") @@ -1266,10 +1298,10 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { return err } -func (t *TableRestore) restoreTable( +func (tr *TableRestore) restoreTable( ctx context.Context, - rc *RestoreController, - cp *TableCheckpoint, + rc *Controller, + cp *checkpoints.TableCheckpoint, ) (bool, error) { // 1. Load the table info. @@ -1281,49 +1313,53 @@ func (t *TableRestore) restoreTable( // no need to do anything if the chunks are already populated if len(cp.Engines) > 0 { - t.logger.Info("reusing engines and files info from checkpoint", + tr.logger.Info("reusing engines and files info from checkpoint", zap.Int("enginesCnt", len(cp.Engines)), zap.Int("filesCnt", cp.CountChunks()), ) - } else if cp.Status < CheckpointStatusAllWritten { - if err := t.populateChunks(ctx, rc, cp); err != nil { + } else if cp.Status < checkpoints.CheckpointStatusAllWritten { + if err := tr.populateChunks(ctx, rc, cp); err != nil { return false, errors.Trace(err) } - if err := rc.checkpointsDB.InsertEngineCheckpoints(ctx, t.tableName, cp.Engines); err != nil { + if err := rc.checkpointsDB.InsertEngineCheckpoints(ctx, tr.tableName, cp.Engines); err != nil { return false, errors.Trace(err) } - web.BroadcastTableCheckpoint(t.tableName, cp) + web.BroadcastTableCheckpoint(tr.tableName, cp) // rebase the allocator so it exceeds the number of rows. - if t.tableInfo.Core.PKIsHandle && t.tableInfo.Core.ContainsAutoRandomBits() { - cp.AllocBase = mathutil.MaxInt64(cp.AllocBase, t.tableInfo.Core.AutoRandID) - t.alloc.Get(autoid.AutoRandomType).Rebase(t.tableInfo.ID, cp.AllocBase, false) + if tr.tableInfo.Core.PKIsHandle && tr.tableInfo.Core.ContainsAutoRandomBits() { + cp.AllocBase = mathutil.MaxInt64(cp.AllocBase, tr.tableInfo.Core.AutoRandID) + if err := tr.alloc.Get(autoid.AutoRandomType).Rebase(tr.tableInfo.ID, cp.AllocBase, false); err != nil { + return false, err + } } else { - cp.AllocBase = mathutil.MaxInt64(cp.AllocBase, t.tableInfo.Core.AutoIncID) - t.alloc.Get(autoid.RowIDAllocType).Rebase(t.tableInfo.ID, cp.AllocBase, false) + cp.AllocBase = mathutil.MaxInt64(cp.AllocBase, tr.tableInfo.Core.AutoIncID) + if err := tr.alloc.Get(autoid.RowIDAllocType).Rebase(tr.tableInfo.ID, cp.AllocBase, false); err != nil { + return false, err + } } rc.saveCpCh <- saveCp{ - tableName: t.tableName, - merger: &RebaseCheckpointMerger{ + tableName: tr.tableName, + merger: &checkpoints.RebaseCheckpointMerger{ AllocBase: cp.AllocBase, }, } } // 2. Restore engines (if still needed) - err := t.restoreEngines(ctx, rc, cp) + err := tr.restoreEngines(ctx, rc, cp) if err != nil { return false, errors.Trace(err) } // 3. Post-process. With the last parameter set to false, we can allow delay analyze execute latter - return t.postProcess(ctx, rc, cp, false /* force-analyze */) + return tr.postProcess(ctx, rc, cp, false /* force-analyze */) } -func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreController, cp *TableCheckpoint) error { +func (tr *TableRestore) restoreEngines(pCtx context.Context, rc *Controller, cp *checkpoints.TableCheckpoint) error { indexEngineCp := cp.Engines[indexEngineID] if indexEngineCp == nil { - return errors.Errorf("table %v index engine checkpoint not found", t.tableName) + return errors.Errorf("table %v index engine checkpoint not found", tr.tableName) } ctx, cancel := context.WithCancel(pCtx) @@ -1341,8 +1377,7 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle // if index-engine checkpoint is lower than `CheckpointStatusClosed`, there must be // data-engines that need to be restore or import. Otherwise, all data-engines should // be finished already. - - if indexEngineCp.Status < CheckpointStatusClosed { + if indexEngineCp.Status < checkpoints.CheckpointStatusClosed { indexWorker := rc.indexWorkers.Apply() defer rc.indexWorkers.Recycle(indexWorker) @@ -1354,8 +1389,8 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle if engineID == indexEngineID { continue } - if engine.Status < CheckpointStatusAllWritten { - indexEngine, err = rc.backend.OpenEngine(ctx, t.tableName, indexEngineID, rc.ts) + if engine.Status < checkpoints.CheckpointStatusAllWritten { + indexEngine, err = rc.backend.OpenEngine(ctx, tr.tableName, indexEngineID, rc.ts) if err != nil { return errors.Trace(err) } @@ -1363,7 +1398,7 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle } } - logTask := t.logger.Begin(zap.InfoLevel, "import whole table") + logTask := tr.logger.Begin(zap.InfoLevel, "import whole table") var wg sync.WaitGroup var engineErr common.OnceError setError := func(err error) { @@ -1374,7 +1409,7 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle type engineCheckpoint struct { engineID int32 - checkpoint *EngineCheckpoint + checkpoint *checkpoints.EngineCheckpoint } allEngines := make([]engineCheckpoint, 0, len(cp.Engines)) for engineID, engine := range cp.Engines { @@ -1402,7 +1437,7 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle continue } - if engine.Status < CheckpointStatusImported { + if engine.Status < checkpoints.CheckpointStatusImported { wg.Add(1) // Note: We still need tableWorkers to control the concurrency of tables. @@ -1410,11 +1445,11 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle // the difference between restoring tables concurrently and restoring tables one by one. restoreWorker := rc.tableWorkers.Apply() - go func(w *worker.Worker, eid int32, ecp *EngineCheckpoint) { + go func(w *worker.Worker, eid int32, ecp *checkpoints.EngineCheckpoint) { defer wg.Done() - engineLogTask := t.logger.With(zap.Int32("engineNumber", eid)).Begin(zap.InfoLevel, "restore engine") - dataClosedEngine, err := t.restoreEngine(ctx, rc, indexEngine, eid, ecp) + engineLogTask := tr.logger.With(zap.Int32("engineNumber", eid)).Begin(zap.InfoLevel, "restore engine") + dataClosedEngine, err := tr.restoreEngine(ctx, rc, indexEngine, eid, ecp) engineLogTask.End(zap.ErrorLevel, err) rc.tableWorkers.Recycle(w) if err != nil { @@ -1428,7 +1463,7 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle dataWorker := rc.closedEngineLimit.Apply() defer rc.closedEngineLimit.Recycle(dataWorker) - if err := t.importEngine(ctx, dataClosedEngine, rc, eid, ecp); err != nil { + if err := tr.importEngine(ctx, dataClosedEngine, rc, eid, ecp); err != nil { setError(err) } }(restoreWorker, engineID, engine) @@ -1446,30 +1481,30 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle if indexEngine != nil { closedIndexEngine, restoreErr = indexEngine.Close(ctx) } else { - closedIndexEngine, restoreErr = rc.backend.UnsafeCloseEngine(ctx, t.tableName, indexEngineID) + closedIndexEngine, restoreErr = rc.backend.UnsafeCloseEngine(ctx, tr.tableName, indexEngineID) } - rc.saveStatusCheckpoint(t.tableName, indexEngineID, restoreErr, CheckpointStatusClosed) - } else if indexEngineCp.Status == CheckpointStatusClosed { + rc.saveStatusCheckpoint(tr.tableName, indexEngineID, restoreErr, checkpoints.CheckpointStatusClosed) + } else if indexEngineCp.Status == checkpoints.CheckpointStatusClosed { // If index engine file has been closed but not imported only if context cancel occurred // when `importKV()` execution, so `UnsafeCloseEngine` and continue import it. - closedIndexEngine, restoreErr = rc.backend.UnsafeCloseEngine(ctx, t.tableName, indexEngineID) + closedIndexEngine, restoreErr = rc.backend.UnsafeCloseEngine(ctx, tr.tableName, indexEngineID) } if restoreErr != nil { return errors.Trace(restoreErr) } - if cp.Status < CheckpointStatusIndexImported { + if cp.Status < checkpoints.CheckpointStatusIndexImported { var err error - if indexEngineCp.Status < CheckpointStatusImported { - err = t.importKV(ctx, closedIndexEngine, rc, indexEngineID) + if indexEngineCp.Status < checkpoints.CheckpointStatusImported { + err = tr.importKV(ctx, closedIndexEngine, rc, indexEngineID) } failpoint.Inject("FailBeforeIndexEngineImported", func() { panic("forcing failure due to FailBeforeIndexEngineImported") }) - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, err, CheckpointStatusIndexImported) + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, err, checkpoints.CheckpointStatusIndexImported) if err != nil { return errors.Trace(err) } @@ -1477,18 +1512,18 @@ func (t *TableRestore) restoreEngines(pCtx context.Context, rc *RestoreControlle return nil } -func (t *TableRestore) restoreEngine( +func (tr *TableRestore) restoreEngine( pCtx context.Context, - rc *RestoreController, + rc *Controller, indexEngine *backend.OpenedEngine, engineID int32, - cp *EngineCheckpoint, + cp *checkpoints.EngineCheckpoint, ) (*backend.ClosedEngine, error) { ctx, cancel := context.WithCancel(pCtx) defer cancel() // all data has finished written, we can close the engine directly. - if cp.Status >= CheckpointStatusAllWritten { - closedEngine, err := rc.backend.UnsafeCloseEngine(ctx, t.tableName, engineID) + if cp.Status >= checkpoints.CheckpointStatusAllWritten { + closedEngine, err := rc.backend.UnsafeCloseEngine(ctx, tr.tableName, engineID) // If any error occurred, recycle worker immediately if err != nil { return closedEngine, errors.Trace(err) @@ -1496,9 +1531,9 @@ func (t *TableRestore) restoreEngine( return closedEngine, nil } - logTask := t.logger.With(zap.Int32("engineNumber", engineID)).Begin(zap.InfoLevel, "encode kv data and write") + logTask := tr.logger.With(zap.Int32("engineNumber", engineID)).Begin(zap.InfoLevel, "encode kv data and write") - dataEngine, err := rc.backend.OpenEngine(ctx, t.tableName, engineID, rc.ts) + dataEngine, err := rc.backend.OpenEngine(ctx, tr.tableName, engineID, rc.ts) if err != nil { return nil, errors.Trace(err) } @@ -1527,7 +1562,7 @@ func (t *TableRestore) restoreEngine( // 2. sql -> kvs // 3. load kvs data (into kv deliver server) // 4. flush kvs data (into tikv node) - cr, err := newChunkRestore(ctx, chunkIndex, rc.cfg, chunk, rc.ioWorkers, rc.store, t.tableInfo) + cr, err := newChunkRestore(ctx, chunkIndex, rc.cfg, chunk, rc.ioWorkers, rc.store, tr.tableInfo) if err != nil { return nil, errors.Trace(err) } @@ -1555,7 +1590,7 @@ func (t *TableRestore) restoreEngine( rc.regionWorkers.Recycle(w) }() metric.ChunkCounter.WithLabelValues(metric.ChunkStateRunning).Add(remainChunkCnt) - err := cr.restore(ctx, t, engineID, dataWriter, indexWriter, rc) + err := cr.restore(ctx, tr, engineID, dataWriter, indexWriter, rc) if err == nil { err = dataWriter.Close() } @@ -1595,7 +1630,7 @@ func (t *TableRestore) restoreEngine( } // Currently we write all the checkpoints after data&index engine are flushed. for _, chunk := range cp.Chunks { - saveCheckpoint(rc, t, engineID, chunk) + saveCheckpoint(rc, tr, engineID, chunk) } return nil } @@ -1604,7 +1639,7 @@ func (t *TableRestore) restoreEngine( // so there may be data lose if exit at here. So we don't write this checkpoint // here like other mode. if !rc.isLocalBackend() { - rc.saveStatusCheckpoint(t.tableName, engineID, err, CheckpointStatusAllWritten) + rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusAllWritten) } if err != nil { // if process is canceled, we should flush all chunk checkpoints for local backend @@ -1631,10 +1666,10 @@ func (t *TableRestore) restoreEngine( // Currently we write all the checkpoints after data&index engine are flushed. for _, chunk := range cp.Chunks { - saveCheckpoint(rc, t, engineID, chunk) + saveCheckpoint(rc, tr, engineID, chunk) } } - rc.saveStatusCheckpoint(t.tableName, engineID, err, CheckpointStatusClosed) + rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusClosed) if err != nil { // If any error occurred, recycle worker immediately return nil, errors.Trace(err) @@ -1642,19 +1677,19 @@ func (t *TableRestore) restoreEngine( return closedDataEngine, nil } -func (t *TableRestore) importEngine( +func (tr *TableRestore) importEngine( ctx context.Context, closedEngine *backend.ClosedEngine, - rc *RestoreController, + rc *Controller, engineID int32, - cp *EngineCheckpoint, + cp *checkpoints.EngineCheckpoint, ) error { - if cp.Status >= CheckpointStatusImported { + if cp.Status >= checkpoints.CheckpointStatusImported { return nil } // 1. calling import - if err := t.importKV(ctx, closedEngine, rc, engineID); err != nil { + if err := tr.importKV(ctx, closedEngine, rc, engineID); err != nil { return errors.Trace(err) } @@ -1676,10 +1711,10 @@ func (t *TableRestore) importEngine( // // if the parameter forcePostProcess to true, postProcess force run checksum and analyze even if the // post-process-at-last config is true. And if this two phases are skipped, the first return value will be true. -func (t *TableRestore) postProcess( +func (tr *TableRestore) postProcess( ctx context.Context, - rc *RestoreController, - cp *TableCheckpoint, + rc *Controller, + cp *checkpoints.TableCheckpoint, forcePostProcess bool, ) (bool, error) { // there are no data in this table, no need to do post process @@ -1690,28 +1725,28 @@ func (t *TableRestore) postProcess( } // 3. alter table set auto_increment - if cp.Status < CheckpointStatusAlteredAutoInc { + if cp.Status < checkpoints.CheckpointStatusAlteredAutoInc { rc.alterTableLock.Lock() - tblInfo := t.tableInfo.Core + tblInfo := tr.tableInfo.Core var err error if tblInfo.PKIsHandle && tblInfo.ContainsAutoRandomBits() { - err = AlterAutoRandom(ctx, rc.tidbGlue.GetSQLExecutor(), t.tableName, t.alloc.Get(autoid.AutoRandomType).Base()+1) + err = AlterAutoRandom(ctx, rc.tidbGlue.GetSQLExecutor(), tr.tableName, tr.alloc.Get(autoid.AutoRandomType).Base()+1) } else if common.TableHasAutoRowID(tblInfo) || tblInfo.GetAutoIncrementColInfo() != nil { // only alter auto increment id iff table contains auto-increment column or generated handle - err = AlterAutoIncrement(ctx, rc.tidbGlue.GetSQLExecutor(), t.tableName, t.alloc.Get(autoid.RowIDAllocType).Base()+1) + err = AlterAutoIncrement(ctx, rc.tidbGlue.GetSQLExecutor(), tr.tableName, tr.alloc.Get(autoid.RowIDAllocType).Base()+1) } rc.alterTableLock.Unlock() - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, err, CheckpointStatusAlteredAutoInc) + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, err, checkpoints.CheckpointStatusAlteredAutoInc) if err != nil { return false, err } - cp.Status = CheckpointStatusAlteredAutoInc + cp.Status = checkpoints.CheckpointStatusAlteredAutoInc } // tidb backend don't need checksum & analyze if !rc.backend.ShouldPostProcess() { - t.logger.Debug("skip checksum & analyze, not supported by this backend") - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, nil, CheckpointStatusAnalyzeSkipped) + tr.logger.Debug("skip checksum & analyze, not supported by this backend") + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, nil, checkpoints.CheckpointStatusAnalyzeSkipped) return false, nil } @@ -1719,10 +1754,10 @@ func (t *TableRestore) postProcess( defer rc.checksumWorks.Recycle(w) finished := true - if cp.Status < CheckpointStatusChecksummed { + if cp.Status < checkpoints.CheckpointStatusChecksummed { if rc.cfg.PostRestore.Checksum == config.OpLevelOff { - t.logger.Info("skip checksum") - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, nil, CheckpointStatusChecksumSkipped) + tr.logger.Info("skip checksum") + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, nil, checkpoints.CheckpointStatusChecksumSkipped) } else { if forcePostProcess || !rc.cfg.PostRestore.PostProcessAtLast { // 4. do table checksum @@ -1732,20 +1767,20 @@ func (t *TableRestore) postProcess( localChecksum.Add(&chunk.Checksum) } } - t.logger.Info("local checksum", zap.Object("checksum", &localChecksum)) - err := t.compareChecksum(ctx, localChecksum) + tr.logger.Info("local checksum", zap.Object("checksum", &localChecksum)) + err := tr.compareChecksum(ctx, localChecksum) // with post restore level 'optional', we will skip checksum error if rc.cfg.PostRestore.Checksum == config.OpLevelOptional { if err != nil { - t.logger.Warn("compare checksum failed, will skip this error and go on", log.ShortError(err)) + tr.logger.Warn("compare checksum failed, will skip this error and go on", log.ShortError(err)) err = nil } } - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, err, CheckpointStatusChecksummed) + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, err, checkpoints.CheckpointStatusChecksummed) if err != nil { return false, errors.Trace(err) } - cp.Status = CheckpointStatusChecksummed + cp.Status = checkpoints.CheckpointStatusChecksummed } else { finished = false } @@ -1756,26 +1791,27 @@ func (t *TableRestore) postProcess( } // 5. do table analyze - if cp.Status < CheckpointStatusAnalyzed { - if rc.cfg.PostRestore.Analyze == config.OpLevelOff { - t.logger.Info("skip analyze") - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, nil, CheckpointStatusAnalyzeSkipped) - cp.Status = CheckpointStatusAnalyzed - } else if forcePostProcess || !rc.cfg.PostRestore.PostProcessAtLast { - err := t.analyzeTable(ctx, rc.tidbGlue.GetSQLExecutor()) + if cp.Status < checkpoints.CheckpointStatusAnalyzed { + switch { + case rc.cfg.PostRestore.Analyze == config.OpLevelOff: + tr.logger.Info("skip analyze") + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, nil, checkpoints.CheckpointStatusAnalyzeSkipped) + cp.Status = checkpoints.CheckpointStatusAnalyzed + case forcePostProcess || !rc.cfg.PostRestore.PostProcessAtLast: + err := tr.analyzeTable(ctx, rc.tidbGlue.GetSQLExecutor()) // witch post restore level 'optional', we will skip analyze error if rc.cfg.PostRestore.Analyze == config.OpLevelOptional { if err != nil { - t.logger.Warn("analyze table failed, will skip this error and go on", log.ShortError(err)) + tr.logger.Warn("analyze table failed, will skip this error and go on", log.ShortError(err)) err = nil } } - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, err, CheckpointStatusAnalyzed) + rc.saveStatusCheckpoint(tr.tableName, checkpoints.WholeTableEngineID, err, checkpoints.CheckpointStatusAnalyzed) if err != nil { return false, errors.Trace(err) } - cp.Status = CheckpointStatusAnalyzed - } else { + cp.Status = checkpoints.CheckpointStatusAnalyzed + default: finished = false } } @@ -1784,7 +1820,7 @@ func (t *TableRestore) postProcess( } // do full compaction for the whole data. -func (rc *RestoreController) fullCompact(ctx context.Context) error { +func (rc *Controller) fullCompact(ctx context.Context) error { if !rc.cfg.PostRestore.Compact { log.L().Info("skip full compaction") return nil @@ -1800,7 +1836,7 @@ func (rc *RestoreController) fullCompact(ctx context.Context) error { return errors.Trace(rc.doCompact(ctx, FullLevelCompact)) } -func (rc *RestoreController) doCompact(ctx context.Context, level int32) error { +func (rc *Controller) doCompact(ctx context.Context, level int32) error { tls := rc.tls.WithHost(rc.cfg.TiDB.PdAddr) return tikv.ForAllStores( ctx, @@ -1812,16 +1848,16 @@ func (rc *RestoreController) doCompact(ctx context.Context, level int32) error { ) } -func (rc *RestoreController) switchToImportMode(ctx context.Context) { +func (rc *Controller) switchToImportMode(ctx context.Context) { rc.switchTiKVMode(ctx, sstpb.SwitchMode_Import) } -func (rc *RestoreController) switchToNormalMode(ctx context.Context) error { +func (rc *Controller) switchToNormalMode(ctx context.Context) error { rc.switchTiKVMode(ctx, sstpb.SwitchMode_Normal) return nil } -func (rc *RestoreController) switchTiKVMode(ctx context.Context, mode sstpb.SwitchMode) { +func (rc *Controller) switchTiKVMode(ctx context.Context, mode sstpb.SwitchMode) { // It is fine if we miss some stores which did not switch to Import mode, // since we're running it periodically, so we exclude disconnected stores. // But it is essential all stores be switched back to Normal mode to allow @@ -1845,7 +1881,7 @@ func (rc *RestoreController) switchTiKVMode(ctx context.Context, mode sstpb.Swit ) } -func (rc *RestoreController) enforceDiskQuota(ctx context.Context) { +func (rc *Controller) enforceDiskQuota(ctx context.Context) { if !atomic.CompareAndSwapInt32(&rc.diskQuotaState, diskQuotaStateIdle, diskQuotaStateChecking) { // do not run multiple the disk quota check / import simultaneously. // (we execute the lock check in background to avoid blocking the cron thread) @@ -1930,7 +1966,7 @@ func (rc *RestoreController) enforceDiskQuota(ctx context.Context) { }() } -func (rc *RestoreController) checkRequirements(ctx context.Context) error { +func (rc *Controller) checkRequirements(ctx context.Context) error { // skip requirement check if explicitly turned off if !rc.cfg.App.CheckRequirements { return nil @@ -1944,7 +1980,7 @@ func (rc *RestoreController) checkRequirements(ctx context.Context) error { return nil } -func (rc *RestoreController) setGlobalVariables(ctx context.Context) error { +func (rc *Controller) setGlobalVariables(ctx context.Context) error { // set new collation flag base on tidb config enabled := ObtainNewCollationEnabled(ctx, rc.tidbGlue.GetSQLExecutor()) // we should enable/disable new collation here since in server mode, tidb config @@ -1953,13 +1989,13 @@ func (rc *RestoreController) setGlobalVariables(ctx context.Context) error { return nil } -func (rc *RestoreController) waitCheckpointFinish() { +func (rc *Controller) waitCheckpointFinish() { // wait checkpoint process finish so that we can do cleanup safely close(rc.saveCpCh) rc.checkpointsWg.Wait() } -func (rc *RestoreController) cleanCheckpoints(ctx context.Context) error { +func (rc *Controller) cleanCheckpoints(ctx context.Context) error { rc.waitCheckpointFinish() if !rc.cfg.Checkpoint.Enable { @@ -1982,24 +2018,24 @@ func (rc *RestoreController) cleanCheckpoints(ctx context.Context) error { return errors.Annotate(err, "clean checkpoints") } -func (rc *RestoreController) isLocalBackend() bool { +func (rc *Controller) isLocalBackend() bool { return rc.cfg.TikvImporter.Backend == "local" } type chunkRestore struct { parser mydump.Parser index int - chunk *ChunkCheckpoint + chunk *checkpoints.ChunkCheckpoint } func newChunkRestore( ctx context.Context, index int, cfg *config.Config, - chunk *ChunkCheckpoint, + chunk *checkpoints.ChunkCheckpoint, ioWorkers *worker.Pool, store storage.ExternalStorage, - tableInfo *TidbTableInfo, + tableInfo *checkpoints.TidbTableInfo, ) (*chunkRestore, error) { blockBufSize := int64(cfg.Mydumper.ReadBlockSize) @@ -2051,8 +2087,8 @@ func (cr *chunkRestore) close() { type TableRestore struct { // The unique table name in the form "`db`.`tbl`". tableName string - dbInfo *TidbDBInfo - tableInfo *TidbTableInfo + dbInfo *checkpoints.TidbDBInfo + tableInfo *checkpoints.TidbTableInfo tableMeta *mydump.MDTableMeta encTable table.Table alloc autoid.Allocators @@ -2062,9 +2098,9 @@ type TableRestore struct { func NewTableRestore( tableName string, tableMeta *mydump.MDTableMeta, - dbInfo *TidbDBInfo, - tableInfo *TidbTableInfo, - cp *TableCheckpoint, + dbInfo *checkpoints.TidbDBInfo, + tableInfo *checkpoints.TidbTableInfo, + cp *checkpoints.TableCheckpoint, ) (*TableRestore, error) { idAlloc := kv.NewPanickingAllocators(cp.AllocBase) tbl, err := tables.TableFromMeta(idAlloc, tableInfo.Core) @@ -2088,9 +2124,9 @@ func (tr *TableRestore) Close() { tr.logger.Info("restore done") } -func (t *TableRestore) populateChunks(ctx context.Context, rc *RestoreController, cp *TableCheckpoint) error { - task := t.logger.Begin(zap.InfoLevel, "load engines and files") - chunks, err := mydump.MakeTableRegions(ctx, t.tableMeta, len(t.tableInfo.Core.Columns), rc.cfg, rc.ioWorkers, rc.store) +func (tr *TableRestore) populateChunks(ctx context.Context, rc *Controller, cp *checkpoints.TableCheckpoint) error { + task := tr.logger.Begin(zap.InfoLevel, "load engines and files") + chunks, err := mydump.MakeTableRegions(ctx, tr.tableMeta, len(tr.tableInfo.Core.Columns), rc.cfg, rc.ioWorkers, rc.store) if err == nil { timestamp := time.Now().Unix() failpoint.Inject("PopulateChunkTimestamp", func(v failpoint.Value) { @@ -2099,13 +2135,13 @@ func (t *TableRestore) populateChunks(ctx context.Context, rc *RestoreController for _, chunk := range chunks { engine, found := cp.Engines[chunk.EngineID] if !found { - engine = &EngineCheckpoint{ - Status: CheckpointStatusLoaded, + engine = &checkpoints.EngineCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, } cp.Engines[chunk.EngineID] = engine } - ccp := &ChunkCheckpoint{ - Key: ChunkCheckpointKey{ + ccp := &checkpoints.ChunkCheckpoint{ + Key: checkpoints.ChunkCheckpointKey{ Path: chunk.FileMeta.Path, Offset: chunk.Chunk.Offset, }, @@ -2115,7 +2151,7 @@ func (t *TableRestore) populateChunks(ctx context.Context, rc *RestoreController Timestamp: timestamp, } if len(chunk.Chunk.Columns) > 0 { - perms, err := t.parseColumnPermutations(chunk.Chunk.Columns) + perms, err := tr.parseColumnPermutations(chunk.Chunk.Columns) if err != nil { return errors.Trace(err) } @@ -2125,7 +2161,7 @@ func (t *TableRestore) populateChunks(ctx context.Context, rc *RestoreController } // Add index engine checkpoint - cp.Engines[indexEngineID] = &EngineCheckpoint{Status: CheckpointStatusLoaded} + cp.Engines[indexEngineID] = &checkpoints.EngineCheckpoint{Status: checkpoints.CheckpointStatusLoaded} } task.End(zap.ErrorLevel, err, zap.Int("enginesCnt", len(cp.Engines)), @@ -2147,14 +2183,14 @@ func (t *TableRestore) populateChunks(ctx context.Context, rc *RestoreController // The column permutation of (d, b, a) is set to be [2, 1, -1, 0]. // // The argument `columns` _must_ be in lower case. -func (t *TableRestore) initializeColumns(columns []string, ccp *ChunkCheckpoint) error { +func (tr *TableRestore) initializeColumns(columns []string, ccp *checkpoints.ChunkCheckpoint) error { var colPerm []int if len(columns) == 0 { - colPerm = make([]int, 0, len(t.tableInfo.Core.Columns)+1) - shouldIncludeRowID := common.TableHasAutoRowID(t.tableInfo.Core) + colPerm = make([]int, 0, len(tr.tableInfo.Core.Columns)+1) + shouldIncludeRowID := common.TableHasAutoRowID(tr.tableInfo.Core) // no provided columns, so use identity permutation. - for i := range t.tableInfo.Core.Columns { + for i := range tr.tableInfo.Core.Columns { colPerm = append(colPerm, i) } if shouldIncludeRowID { @@ -2162,7 +2198,7 @@ func (t *TableRestore) initializeColumns(columns []string, ccp *ChunkCheckpoint) } } else { var err error - colPerm, err = t.parseColumnPermutations(columns) + colPerm, err = tr.parseColumnPermutations(columns) if err != nil { return errors.Trace(err) } @@ -2172,8 +2208,8 @@ func (t *TableRestore) initializeColumns(columns []string, ccp *ChunkCheckpoint) return nil } -func (t *TableRestore) parseColumnPermutations(columns []string) ([]int, error) { - colPerm := make([]int, 0, len(t.tableInfo.Core.Columns)+1) +func (tr *TableRestore) parseColumnPermutations(columns []string) ([]int, error) { + colPerm := make([]int, 0, len(tr.tableInfo.Core.Columns)+1) columnMap := make(map[string]int) for i, column := range columns { @@ -2181,7 +2217,7 @@ func (t *TableRestore) parseColumnPermutations(columns []string) ([]int, error) } tableColumnMap := make(map[string]int) - for i, col := range t.tableInfo.Core.Columns { + for i, col := range tr.tableInfo.Core.Columns { tableColumnMap[col.Name.L] = i } @@ -2196,12 +2232,12 @@ func (t *TableRestore) parseColumnPermutations(columns []string) ([]int, error) return colPerm, errors.Errorf("unknown columns in header %s", unknownCols) } - for _, colInfo := range t.tableInfo.Core.Columns { + for _, colInfo := range tr.tableInfo.Core.Columns { if i, ok := columnMap[colInfo.Name.L]; ok { colPerm = append(colPerm, i) } else { if len(colInfo.GeneratedExprString) == 0 { - t.logger.Warn("column missing from data file, going to fill with default value", + tr.logger.Warn("column missing from data file, going to fill with default value", zap.String("colName", colInfo.Name.O), zap.Stringer("colType", &colInfo.FieldType), ) @@ -2211,7 +2247,7 @@ func (t *TableRestore) parseColumnPermutations(columns []string) ([]int, error) } if i, ok := columnMap[model.ExtraHandleName.L]; ok { colPerm = append(colPerm, i) - } else if common.TableHasAutoRowID(t.tableInfo.Core) { + } else if common.TableHasAutoRowID(tr.tableInfo.Core) { colPerm = append(colPerm, -1) } @@ -2235,7 +2271,7 @@ func getColumnNames(tableInfo *model.TableInfo, permutation []int) []string { for _, idx := range colIndexes { // skip columns with index -1 if idx >= 0 { - // original fiels contains _tidb_rowid field + // original fields contains _tidb_rowid field if idx == len(tableInfo.Columns) { names = append(names, model.ExtraHandleName.O) } else { @@ -2249,15 +2285,15 @@ func getColumnNames(tableInfo *model.TableInfo, permutation []int) []string { func (tr *TableRestore) importKV( ctx context.Context, closedEngine *backend.ClosedEngine, - rc *RestoreController, + rc *Controller, engineID int32, ) error { task := closedEngine.Logger().Begin(zap.InfoLevel, "import and cleanup engine") err := closedEngine.Import(ctx) - rc.saveStatusCheckpoint(tr.tableName, engineID, err, CheckpointStatusImported) + rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusImported) if err == nil { - closedEngine.Cleanup(ctx) + err = closedEngine.Cleanup(ctx) } dur := task.End(zap.ErrorLevel, err) @@ -2301,8 +2337,6 @@ func (tr *TableRestore) analyzeTable(ctx context.Context, g glue.SQLExecutor) er return err } -//////////////////////////////////////////////////////////////// - var ( maxKVQueueSize = 128 // Cache at most this number of rows before blocking the encode loop minDeliverBytes uint64 = 96 * units.KiB // 96 KB (data + index). batch at least this amount of bytes to reduce number of messages @@ -2320,13 +2354,14 @@ type deliverResult struct { err error } +//nolint:nakedret // TODO: refactor func (cr *chunkRestore) deliverLoop( ctx context.Context, kvsCh <-chan []deliveredKVs, t *TableRestore, engineID int32, dataEngine, indexEngine *backend.LocalEngineWriter, - rc *RestoreController, + rc *Controller, ) (deliverTotalDur time.Duration, err error) { var channelClosed bool @@ -2420,7 +2455,7 @@ func (cr *chunkRestore) deliverLoop( deliverLogger.Warn("Slowed down write rows") }) failpoint.Inject("FailAfterWriteRows", nil) - // TODO: for local backend, we may save checkpoint more frequently, e.g. after writen + // TODO: for local backend, we may save checkpoint more frequently, e.g. after written // 10GB kv pairs to data engine, we can do a flush for both data & index engine, then we // can safely update current checkpoint. @@ -2435,7 +2470,7 @@ func (cr *chunkRestore) deliverLoop( return } -func saveCheckpoint(rc *RestoreController, t *TableRestore, engineID int32, chunk *ChunkCheckpoint) { +func saveCheckpoint(rc *Controller, t *TableRestore, engineID int32, chunk *checkpoints.ChunkCheckpoint) { // We need to update the AllocBase every time we've finished a file. // The AllocBase is determined by the maximum of the "handle" (_tidb_rowid // or integer primary key), which can only be obtained by reading all data. @@ -2448,13 +2483,13 @@ func saveCheckpoint(rc *RestoreController, t *TableRestore, engineID int32, chun } rc.saveCpCh <- saveCp{ tableName: t.tableName, - merger: &RebaseCheckpointMerger{ + merger: &checkpoints.RebaseCheckpointMerger{ AllocBase: base, }, } rc.saveCpCh <- saveCp{ tableName: t.tableName, - merger: &ChunkCheckpointMerger{ + merger: &checkpoints.ChunkCheckpointMerger{ EngineID: engineID, Key: chunk.Key, Checksum: chunk.Checksum, @@ -2465,6 +2500,7 @@ func saveCheckpoint(rc *RestoreController, t *TableRestore, engineID int32, chun } } +//nolint:nakedret // TODO: refactor func (cr *chunkRestore) encodeLoop( ctx context.Context, kvsCh chan<- []deliveredKVs, @@ -2472,7 +2508,7 @@ func (cr *chunkRestore) encodeLoop( logger log.Logger, kvEncoder kv.Encoder, deliverCompleteCh <-chan deliverResult, - rc *RestoreController, + rc *Controller, ) (readTotalDur time.Duration, encodeTotalDur time.Duration, err error) { send := func(kvs []deliveredKVs) error { select { @@ -2570,7 +2606,7 @@ func (cr *chunkRestore) restore( t *TableRestore, engineID int32, dataEngine, indexEngine *backend.LocalEngineWriter, - rc *RestoreController, + rc *Controller, ) error { // Create the encoder. kvEncoder, err := rc.backend.NewEncoder(t.encTable, &kv.SessionOptions{ diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index 94542bdc0..a366f54e9 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -41,7 +41,6 @@ import ( "github.com/pingcap/br/pkg/lightning/backend/noop" "github.com/pingcap/br/pkg/lightning/backend/tidb" "github.com/pingcap/br/pkg/lightning/checkpoints" - . "github.com/pingcap/br/pkg/lightning/checkpoints" "github.com/pingcap/br/pkg/lightning/common" "github.com/pingcap/br/pkg/lightning/config" "github.com/pingcap/br/pkg/lightning/glue" @@ -73,7 +72,7 @@ func (s *restoreSuite) TestNewTableRestore(c *C) { p := parser.New() se := tmock.NewContext() - dbInfo := &TidbDBInfo{Name: "mockdb", Tables: map[string]*TidbTableInfo{}} + dbInfo := &checkpoints.TidbDBInfo{Name: "mockdb", Tables: map[string]*checkpoints.TidbTableInfo{}} for i, tc := range testCases { node, err := p.ParseOneStmt(tc.createStmt, "utf8mb4", "utf8mb4_bin") c.Assert(err, IsNil) @@ -81,7 +80,7 @@ func (s *restoreSuite) TestNewTableRestore(c *C) { c.Assert(err, IsNil) tableInfo.State = model.StatePublic - dbInfo.Tables[tc.name] = &TidbTableInfo{ + dbInfo.Tables[tc.name] = &checkpoints.TidbTableInfo{ Name: tc.name, Core: tableInfo, } @@ -90,23 +89,23 @@ func (s *restoreSuite) TestNewTableRestore(c *C) { for _, tc := range testCases { tableInfo := dbInfo.Tables[tc.name] tableName := common.UniqueTable("mockdb", tableInfo.Name) - tr, err := NewTableRestore(tableName, nil, dbInfo, tableInfo, &TableCheckpoint{}) + tr, err := NewTableRestore(tableName, nil, dbInfo, tableInfo, &checkpoints.TableCheckpoint{}) c.Assert(tr, NotNil) c.Assert(err, IsNil) } } func (s *restoreSuite) TestNewTableRestoreFailure(c *C) { - tableInfo := &TidbTableInfo{ + tableInfo := &checkpoints.TidbTableInfo{ Name: "failure", Core: &model.TableInfo{}, } - dbInfo := &TidbDBInfo{Name: "mockdb", Tables: map[string]*TidbTableInfo{ + dbInfo := &checkpoints.TidbDBInfo{Name: "mockdb", Tables: map[string]*checkpoints.TidbTableInfo{ "failure": tableInfo, }} tableName := common.UniqueTable("mockdb", "failure") - _, err := NewTableRestore(tableName, nil, dbInfo, tableInfo, &TableCheckpoint{}) + _, err := NewTableRestore(tableName, nil, dbInfo, tableInfo, &checkpoints.TableCheckpoint{}) c.Assert(err, ErrorMatches, `failed to tables\.TableFromMeta.*`) } @@ -114,8 +113,8 @@ func (s *restoreSuite) TestErrorSummaries(c *C) { logger, buffer := log.MakeTestLogger() es := makeErrorSummaries(logger) - es.record("first", errors.New("a1 error"), CheckpointStatusAnalyzed) - es.record("second", errors.New("b2 error"), CheckpointStatusAllWritten) + es.record("first", errors.New("a1 error"), checkpoints.CheckpointStatusAnalyzed) + es.record("second", errors.New("b2 error"), checkpoints.CheckpointStatusAllWritten) es.emitLog() lines := buffer.Lines() @@ -214,8 +213,8 @@ type tableRestoreSuiteBase struct { tr *TableRestore cfg *config.Config - tableInfo *TidbTableInfo - dbInfo *TidbDBInfo + tableInfo *checkpoints.TidbTableInfo + dbInfo *checkpoints.TidbDBInfo tableMeta *mydump.MDTableMeta store storage.ExternalStorage @@ -244,10 +243,10 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { c.Assert(err, IsNil) core.State = model.StatePublic - s.tableInfo = &TidbTableInfo{Name: "table", DB: "db", Core: core} - s.dbInfo = &TidbDBInfo{ + s.tableInfo = &checkpoints.TidbTableInfo{Name: "table", DB: "db", Core: core} + s.dbInfo = &checkpoints.TidbDBInfo{ Name: "db", - Tables: map[string]*TidbTableInfo{"table": s.tableInfo}, + Tables: map[string]*checkpoints.TidbTableInfo{"table": s.tableInfo}, } // Write some sample SQL dump @@ -267,28 +266,50 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { fakeDataPath := filepath.Join(fakeDataDir, fakeFileName) err = ioutil.WriteFile(fakeDataPath, fakeDataFilesContent, 0o644) c.Assert(err, IsNil) - fakeDataFiles = append(fakeDataFiles, mydump.FileInfo{TableName: filter.Table{"db", "table"}, FileMeta: mydump.SourceFileMeta{Path: fakeFileName, Type: mydump.SourceTypeSQL, SortKey: fmt.Sprintf("%d", i), FileSize: 37}}) + fakeDataFiles = append(fakeDataFiles, mydump.FileInfo{ + TableName: filter.Table{Schema: "db", Name: "table"}, + FileMeta: mydump.SourceFileMeta{ + Path: fakeFileName, + Type: mydump.SourceTypeSQL, + SortKey: fmt.Sprintf("%d", i), + FileSize: 37, + }, + }) } fakeCsvContent := []byte("1,2,3\r\n4,5,6\r\n") csvName := "db.table.99.csv" err = ioutil.WriteFile(filepath.Join(fakeDataDir, csvName), fakeCsvContent, 0o644) c.Assert(err, IsNil) - fakeDataFiles = append(fakeDataFiles, mydump.FileInfo{TableName: filter.Table{"db", "table"}, FileMeta: mydump.SourceFileMeta{Path: csvName, Type: mydump.SourceTypeCSV, SortKey: "99", FileSize: 14}}) + fakeDataFiles = append(fakeDataFiles, mydump.FileInfo{ + TableName: filter.Table{Schema: "db", Name: "table"}, + FileMeta: mydump.SourceFileMeta{ + Path: csvName, + Type: mydump.SourceTypeCSV, + SortKey: "99", + FileSize: 14, + }, + }) s.tableMeta = &mydump.MDTableMeta{ - DB: "db", - Name: "table", - TotalSize: 222, - SchemaFile: mydump.FileInfo{TableName: filter.Table{Schema: "db", Name: "table"}, FileMeta: mydump.SourceFileMeta{Path: "db.table-schema.sql", Type: mydump.SourceTypeTableSchema}}, - DataFiles: fakeDataFiles, + DB: "db", + Name: "table", + TotalSize: 222, + SchemaFile: mydump.FileInfo{ + TableName: filter.Table{Schema: "db", Name: "table"}, + FileMeta: mydump.SourceFileMeta{ + Path: "db.table-schema.sql", + Type: mydump.SourceTypeTableSchema, + }, + }, + DataFiles: fakeDataFiles, } } func (s *tableRestoreSuiteBase) SetUpTest(c *C) { // Collect into the test TableRestore structure var err error - s.tr, err = NewTableRestore("`db`.`table`", s.tableMeta, s.dbInfo, s.tableInfo, &TableCheckpoint{}) + s.tr, err = NewTableRestore("`db`.`table`", s.tableMeta, s.dbInfo, s.tableInfo, &checkpoints.TableCheckpoint{}) c.Assert(err, IsNil) s.cfg = config.NewConfig() @@ -297,25 +318,28 @@ func (s *tableRestoreSuiteBase) SetUpTest(c *C) { } func (s *tableRestoreSuite) TestPopulateChunks(c *C) { - failpoint.Enable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp", "return(1234567897)") - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp") + _ = failpoint.Enable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp", "return(1234567897)") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp") + }() - cp := &TableCheckpoint{ - Engines: make(map[int32]*EngineCheckpoint), + cp := &checkpoints.TableCheckpoint{ + Engines: make(map[int32]*checkpoints.EngineCheckpoint), } - rc := &RestoreController{cfg: s.cfg, ioWorkers: worker.NewPool(context.Background(), 1, "io"), store: s.store} + rc := &Controller{cfg: s.cfg, ioWorkers: worker.NewPool(context.Background(), 1, "io"), store: s.store} err := s.tr.populateChunks(context.Background(), rc, cp) c.Assert(err, IsNil) - c.Assert(cp.Engines, DeepEquals, map[int32]*EngineCheckpoint{ + //nolint:dupl // false positive. + c.Assert(cp.Engines, DeepEquals, map[int32]*checkpoints.EngineCheckpoint{ -1: { - Status: CheckpointStatusLoaded, + Status: checkpoints.CheckpointStatusLoaded, }, 0: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[0].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[0].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[0].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -326,7 +350,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[1].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -337,7 +361,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[2].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[2].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[2].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -350,10 +374,10 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { }, }, 1: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[3].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[3].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[3].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -364,7 +388,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[4].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[4].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[4].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -375,7 +399,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[5].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[5].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[5].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -388,10 +412,10 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { }, }, 2: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[6].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[6].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[6].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -442,7 +466,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { err := ioutil.WriteFile(filepath.Join(fakeDataDir, csvName), []byte(s), 0o644) c.Assert(err, IsNil) fakeDataFiles = append(fakeDataFiles, mydump.FileInfo{ - TableName: filter.Table{"db", "table"}, + TableName: filter.Table{Schema: "db", Name: "table"}, FileMeta: mydump.SourceFileMeta{Path: csvName, Type: mydump.SourceTypeCSV, SortKey: fmt.Sprintf("%02d", i), FileSize: int64(len(s))}, }) total += len(s) @@ -455,11 +479,13 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { DataFiles: fakeDataFiles, } - failpoint.Enable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp", "return(1234567897)") - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp") + _ = failpoint.Enable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp", "return(1234567897)") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/restore/PopulateChunkTimestamp") + }() - cp := &TableCheckpoint{ - Engines: make(map[int32]*EngineCheckpoint), + cp := &checkpoints.TableCheckpoint{ + Engines: make(map[int32]*checkpoints.EngineCheckpoint), } cfg := config.NewConfig() @@ -468,21 +494,21 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { cfg.Mydumper.CSV.Header = true cfg.Mydumper.StrictFormat = true - rc := &RestoreController{cfg: cfg, ioWorkers: worker.NewPool(context.Background(), 1, "io"), store: store} + rc := &Controller{cfg: cfg, ioWorkers: worker.NewPool(context.Background(), 1, "io"), store: store} - tr, err := NewTableRestore("`db`.`table`", tableMeta, s.dbInfo, s.tableInfo, &TableCheckpoint{}) + tr, err := NewTableRestore("`db`.`table`", tableMeta, s.dbInfo, s.tableInfo, &checkpoints.TableCheckpoint{}) c.Assert(err, IsNil) c.Assert(tr.populateChunks(context.Background(), rc, cp), IsNil) - c.Assert(cp.Engines, DeepEquals, map[int32]*EngineCheckpoint{ + c.Assert(cp.Engines, DeepEquals, map[int32]*checkpoints.EngineCheckpoint{ -1: { - Status: CheckpointStatusLoaded, + Status: checkpoints.CheckpointStatusLoaded, }, 0: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[0].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[0].FileMeta.Path, Offset: 0}, FileMeta: tableMeta.DataFiles[0].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -493,7 +519,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, FileMeta: tableMeta.DataFiles[1].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -504,7 +530,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[2].FileMeta.Path, Offset: 6}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[2].FileMeta.Path, Offset: 6}, FileMeta: tableMeta.DataFiles[2].FileMeta, ColumnPermutation: []int{0, 1, 2, -1}, Chunk: mydump.Chunk{ @@ -518,7 +544,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[2].FileMeta.Path, Offset: 52}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[2].FileMeta.Path, Offset: 52}, FileMeta: tableMeta.DataFiles[2].FileMeta, ColumnPermutation: []int{0, 1, 2, -1}, Chunk: mydump.Chunk{ @@ -531,7 +557,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 6}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 6}, FileMeta: tableMeta.DataFiles[3].FileMeta, ColumnPermutation: []int{1, 2, 0, -1}, Chunk: mydump.Chunk{ @@ -546,10 +572,10 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { }, }, 1: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 48}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 48}, FileMeta: tableMeta.DataFiles[3].FileMeta, ColumnPermutation: []int{1, 2, 0, -1}, Chunk: mydump.Chunk{ @@ -562,7 +588,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 101}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[3].FileMeta.Path, Offset: 101}, FileMeta: tableMeta.DataFiles[3].FileMeta, ColumnPermutation: []int{1, 2, 0, -1}, Chunk: mydump.Chunk{ @@ -575,7 +601,7 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { Timestamp: 1234567897, }, { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[4].FileMeta.Path, Offset: 4}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[4].FileMeta.Path, Offset: 4}, FileMeta: tableMeta.DataFiles[4].FileMeta, ColumnPermutation: []int{-1, 0, 1, -1}, Chunk: mydump.Chunk{ @@ -590,10 +616,10 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { }, }, 2: { - Status: CheckpointStatusLoaded, - Chunks: []*ChunkCheckpoint{ + Status: checkpoints.CheckpointStatusLoaded, + Chunks: []*checkpoints.ChunkCheckpoint{ { - Key: ChunkCheckpointKey{Path: tableMeta.DataFiles[4].FileMeta.Path, Offset: 59}, + Key: checkpoints.ChunkCheckpointKey{Path: tableMeta.DataFiles[4].FileMeta.Path, Offset: 59}, FileMeta: tableMeta.DataFiles[4].FileMeta, ColumnPermutation: []int{-1, 0, 1, -1}, Chunk: mydump.Chunk{ @@ -625,7 +651,7 @@ func (s *tableRestoreSuite) TestGetColumnsNames(c *C) { } func (s *tableRestoreSuite) TestInitializeColumns(c *C) { - ccp := &ChunkCheckpoint{} + ccp := &checkpoints.ChunkCheckpoint{} c.Assert(s.tr.initializeColumns(nil, ccp), IsNil) c.Assert(ccp.ColumnPermutation, DeepEquals, []int{0, 1, 2, -1}) @@ -732,7 +758,7 @@ func (s *tableRestoreSuite) TestImportKVSuccess(c *C) { importer := backend.MakeBackend(mockBackend) chptCh := make(chan saveCp) defer close(chptCh) - rc := &RestoreController{saveCpCh: chptCh} + rc := &Controller{saveCpCh: chptCh} go func() { for range chptCh { } @@ -764,7 +790,7 @@ func (s *tableRestoreSuite) TestImportKVFailure(c *C) { importer := backend.MakeBackend(mockBackend) chptCh := make(chan saveCp) defer close(chptCh) - rc := &RestoreController{saveCpCh: chptCh} + rc := &Controller{saveCpCh: chptCh} go func() { for range chptCh { } @@ -797,7 +823,7 @@ func (s *tableRestoreSuite) TestCheckRequirements(c *C) { mockBackend.EXPECT(). CheckRequirements(ctx, gomock.Any()). Return(errors.Annotate(context.Canceled, "fake check requirement error")) - rc := &RestoreController{ + rc := &Controller{ cfg: &config.Config{App: config.Lightning{CheckRequirements: true}}, backend: backend, } @@ -837,7 +863,7 @@ func (s *tableRestoreSuite) TestTableRestoreMetrics(c *C) { c.Assert(err, IsNil) cpDB := checkpoints.NewNullCheckpointsDB() - rc := &RestoreController{ + rc := &Controller{ cfg: cfg, dbMetas: []*mydump.MDDatabaseMeta{ { @@ -845,7 +871,7 @@ func (s *tableRestoreSuite) TestTableRestoreMetrics(c *C) { Tables: []*mydump.MDTableMeta{s.tableMeta}, }, }, - dbInfos: map[string]*TidbDBInfo{ + dbInfos: map[string]*checkpoints.TidbDBInfo{ s.tableInfo.DB: s.dbInfo, }, tableWorkers: worker.NewPool(ctx, 6, "table"), @@ -898,8 +924,8 @@ func (s *chunkRestoreSuite) SetUpTest(c *C) { ctx := context.Background() w := worker.NewPool(ctx, 5, "io") - chunk := ChunkCheckpoint{ - Key: ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, + chunk := checkpoints.ChunkCheckpoint{ + Key: checkpoints.ChunkCheckpointKey{Path: s.tr.tableMeta.DataFiles[1].FileMeta.Path, Offset: 0}, FileMeta: s.tr.tableMeta.DataFiles[1].FileMeta, Chunk: mydump.Chunk{ Offset: 0, @@ -919,8 +945,7 @@ func (s *chunkRestoreSuite) TearDownTest(c *C) { } func (s *chunkRestoreSuite) TestDeliverLoopCancel(c *C) { - rc := &RestoreController{backend: importer.NewMockImporter(nil, "")} - + rc := &Controller{backend: importer.NewMockImporter(nil, "")} ctx, cancel := context.WithCancel(context.Background()) kvsCh := make(chan []deliveredKVs) go cancel() @@ -958,7 +983,7 @@ func (s *chunkRestoreSuite) TestDeliverLoopEmptyData(c *C) { // Deliver nothing. cfg := &config.Config{} - rc := &RestoreController{cfg: cfg, backend: importer} + rc := &Controller{cfg: cfg, backend: importer} kvsCh := make(chan []deliveredKVs, 1) kvsCh <- []deliveredKVs{} @@ -1051,7 +1076,7 @@ func (s *chunkRestoreSuite) TestDeliverLoop(c *C) { }() cfg := &config.Config{} - rc := &RestoreController{cfg: cfg, saveCpCh: saveCpCh, backend: importer} + rc := &Controller{cfg: cfg, saveCpCh: saveCpCh, backend: importer} _, err = s.cr.deliverLoop(ctx, kvsCh, s.tr, 0, dataWriter, indexWriter, rc) c.Assert(err, IsNil) @@ -1071,7 +1096,7 @@ func (s *chunkRestoreSuite) TestEncodeLoop(c *C) { }) c.Assert(err, IsNil) cfg := config.NewConfig() - rc := &RestoreController{pauser: DeliverPauser, cfg: cfg} + rc := &Controller{pauser: DeliverPauser, cfg: cfg} _, _, err = s.cr.encodeLoop(ctx, kvsCh, s.tr, s.tr.logger, kvEncoder, deliverCompleteCh, rc) c.Assert(err, IsNil) c.Assert(kvsCh, HasLen, 2) @@ -1098,7 +1123,7 @@ func (s *chunkRestoreSuite) TestEncodeLoopCanceled(c *C) { go cancel() cfg := config.NewConfig() - rc := &RestoreController{pauser: DeliverPauser, cfg: cfg} + rc := &Controller{pauser: DeliverPauser, cfg: cfg} _, _, err = s.cr.encodeLoop(ctx, kvsCh, s.tr, s.tr.logger, kvEncoder, deliverCompleteCh, rc) c.Assert(errors.Cause(err), Equals, context.Canceled) c.Assert(kvsCh, HasLen, 0) @@ -1118,7 +1143,7 @@ func (s *chunkRestoreSuite) TestEncodeLoopForcedError(c *C) { s.cr.parser.Close() cfg := config.NewConfig() - rc := &RestoreController{pauser: DeliverPauser, cfg: cfg} + rc := &Controller{pauser: DeliverPauser, cfg: cfg} _, _, err = s.cr.encodeLoop(ctx, kvsCh, s.tr, s.tr.logger, kvEncoder, deliverCompleteCh, rc) c.Assert(err, ErrorMatches, `in file .*[/\\]?db\.table\.2\.sql:0 at offset 0:.*file already closed`) c.Assert(kvsCh, HasLen, 0) @@ -1140,7 +1165,7 @@ func (s *chunkRestoreSuite) TestEncodeLoopDeliverErrored(c *C) { } }() cfg := config.NewConfig() - rc := &RestoreController{pauser: DeliverPauser, cfg: cfg} + rc := &Controller{pauser: DeliverPauser, cfg: cfg} _, _, err = s.cr.encodeLoop(ctx, kvsCh, s.tr, s.tr.logger, kvEncoder, deliverCompleteCh, rc) c.Assert(err, ErrorMatches, "fake deliver error") c.Assert(kvsCh, HasLen, 0) @@ -1157,7 +1182,7 @@ func (s *chunkRestoreSuite) TestEncodeLoopColumnsMismatch(c *C) { ctx := context.Background() cfg := config.NewConfig() - rc := &RestoreController{pauser: DeliverPauser, cfg: cfg} + rc := &Controller{pauser: DeliverPauser, cfg: cfg} reader, err := store.Open(ctx, fileName) c.Assert(err, IsNil) @@ -1229,7 +1254,7 @@ func (s *chunkRestoreSuite) TestRestore(c *C) { // Now actually start the restore loop. saveCpCh := make(chan saveCp, 2) - err = s.cr.restore(ctx, s.tr, 0, dataWriter, indexWriter, &RestoreController{ + err = s.cr.restore(ctx, s.tr, 0, dataWriter, indexWriter, &Controller{ cfg: s.cfg, saveCpCh: saveCpCh, backend: importer, @@ -1243,7 +1268,7 @@ var _ = Suite(&restoreSchemaSuite{}) type restoreSchemaSuite struct { ctx context.Context - rc *RestoreController + rc *Controller controller *gomock.Controller } @@ -1285,7 +1310,7 @@ func (s *restoreSchemaSuite) SetUpSuite(c *C) { config.App.RegionConcurrency = 8 mydumpLoader, err := mydump.NewMyDumpLoaderWithStore(ctx, config, store) c.Assert(err, IsNil) - s.rc = &RestoreController{ + s.rc = &Controller{ cfg: config, store: store, dbMetas: mydumpLoader.GetDatabases(), @@ -1293,6 +1318,7 @@ func (s *restoreSchemaSuite) SetUpSuite(c *C) { } } +//nolint:interfacer // change test case signature might cause Check failed to find this test case? func (s *restoreSchemaSuite) SetUpTest(c *C) { s.controller, s.ctx = gomock.WithContext(context.Background(), c) mockBackend := mock.NewMockBackend(s.controller) diff --git a/pkg/lightning/restore/tidb.go b/pkg/lightning/restore/tidb.go index 18b7713a4..3ffc05fdf 100644 --- a/pkg/lightning/restore/tidb.go +++ b/pkg/lightning/restore/tidb.go @@ -31,7 +31,7 @@ import ( "github.com/pingcap/br/pkg/lightning/glue" - . "github.com/pingcap/br/pkg/lightning/checkpoints" + "github.com/pingcap/br/pkg/lightning/checkpoints" "github.com/pingcap/br/pkg/lightning/common" "github.com/pingcap/br/pkg/lightning/config" "github.com/pingcap/br/pkg/lightning/log" @@ -236,17 +236,17 @@ func LoadSchemaInfo( ctx context.Context, schemas []*mydump.MDDatabaseMeta, getTables func(context.Context, string) ([]*model.TableInfo, error), -) (map[string]*TidbDBInfo, error) { - result := make(map[string]*TidbDBInfo, len(schemas)) +) (map[string]*checkpoints.TidbDBInfo, error) { + result := make(map[string]*checkpoints.TidbDBInfo, len(schemas)) for _, schema := range schemas { tables, err := getTables(ctx, schema.Name) if err != nil { return nil, err } - dbInfo := &TidbDBInfo{ + dbInfo := &checkpoints.TidbDBInfo{ Name: schema.Name, - Tables: make(map[string]*TidbTableInfo), + Tables: make(map[string]*checkpoints.TidbTableInfo), } for _, tbl := range tables { @@ -260,7 +260,7 @@ func LoadSchemaInfo( if err != nil { return nil, errors.Trace(err) } - tableInfo := &TidbTableInfo{ + tableInfo := &checkpoints.TidbTableInfo{ ID: tbl.ID, DB: schema.Name, Name: tableName, diff --git a/pkg/lightning/restore/tidb_test.go b/pkg/lightning/restore/tidb_test.go index fab65d679..c9f7ff021 100644 --- a/pkg/lightning/restore/tidb_test.go +++ b/pkg/lightning/restore/tidb_test.go @@ -15,7 +15,6 @@ package restore import ( "context" - "net/http" "testing" "github.com/DATA-DOG/go-sqlmock" @@ -37,10 +36,9 @@ import ( var _ = Suite(&tidbSuite{}) type tidbSuite struct { - mockDB sqlmock.Sqlmock - handler http.Handler - timgr *TiDBManager - tiGlue glue.Glue + mockDB sqlmock.Sqlmock + timgr *TiDBManager + tiGlue glue.Glue } func TestTiDB(t *testing.T) { diff --git a/pkg/lightning/tikv/tikv_test.go b/pkg/lightning/tikv/tikv_test.go index 72a89f92c..aa5467722 100644 --- a/pkg/lightning/tikv/tikv_test.go +++ b/pkg/lightning/tikv/tikv_test.go @@ -33,7 +33,7 @@ var ( func (s *tikvSuite) TestForAllStores(c *C) { server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - w.Write([]byte(` + _, err := w.Write([]byte(` { "count": 5, "stores": [ @@ -85,6 +85,7 @@ func (s *tikvSuite) TestForAllStores(c *C) { ] } `)) + c.Assert(err, IsNil) })) defer server.Close() diff --git a/pkg/lightning/web/progress.go b/pkg/lightning/web/progress.go index 099594dda..d109f7ed3 100644 --- a/pkg/lightning/web/progress.go +++ b/pkg/lightning/web/progress.go @@ -88,9 +88,8 @@ func (cpm *checkpointsMap) marshal(key string) ([]byte, error) { type taskStatus uint8 const ( - taskStatusNotStarted taskStatus = 0 - taskStatusRunning = 1 - taskStatusCompleted = 2 + taskStatusRunning taskStatus = 1 + taskStatusCompleted taskStatus = 2 ) type tableInfo struct { diff --git a/pkg/mock/glue.go b/pkg/mock/glue.go index 63ece855e..875dfa6e8 100644 --- a/pkg/mock/glue.go +++ b/pkg/mock/glue.go @@ -128,10 +128,10 @@ func (mr *MockGlueMockRecorder) GetSession(arg0 interface{}) *gomock.Call { } // OpenCheckpointsDB mocks base method -func (m *MockGlue) OpenCheckpointsDB(arg0 context.Context, arg1 *config.Config) (checkpoints.CheckpointsDB, error) { +func (m *MockGlue) OpenCheckpointsDB(arg0 context.Context, arg1 *config.Config) (checkpoints.DB, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "OpenCheckpointsDB", arg0, arg1) - ret0, _ := ret[0].(checkpoints.CheckpointsDB) + ret0, _ := ret[0].(checkpoints.DB) ret1, _ := ret[1].(error) return ret0, ret1 }