From 2b58575c9a7b986296786123a878a701bb644207 Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 16:23:19 +0800 Subject: [PATCH 01/29] *: fix lightning unparam --- cmd/tidb-lightning-ctl/main_test.go | 2 +- cmd/tidb-lightning/main_test.go | 2 +- pkg/lightning/backend/tidb.go | 2 +- pkg/lightning/common/util.go | 2 +- pkg/lightning/common/util_test.go | 4 ++-- pkg/lightning/mydump/loader_test.go | 5 ++--- 6 files changed, 8 insertions(+), 9 deletions(-) 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_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/tidb.go b/pkg/lightning/backend/tidb.go index d9f64bb13..4a90f54ed 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -157,7 +157,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") diff --git a/pkg/lightning/common/util.go b/pkg/lightning/common/util.go index 562335e29..c0b6b17c6 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/mydump/loader_test.go b/pkg/lightning/mydump/loader_test.go index 816e904d8..c29a6759a 100644 --- a/pkg/lightning/mydump/loader_test.go +++ b/pkg/lightning/mydump/loader_test.go @@ -58,14 +58,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) { @@ -481,7 +480,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") From f50fac744fb0421de7b82f6fe0d2235ac9aedbce Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 16:25:26 +0800 Subject: [PATCH 02/29] *: fix lightning maligned --- pkg/lightning/config/config.go | 14 +++++++------- pkg/lightning/mydump/csv_parser.go | 12 ++++++------ pkg/lightning/restore/restore.go | 10 +++++----- 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index 69b64ec2e..3dc2f4ea9 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -233,20 +233,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"` } @@ -255,14 +255,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"` } @@ -289,10 +289,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"` } diff --git a/pkg/lightning/mydump/csv_parser.go b/pkg/lightning/mydump/csv_parser.go index 1e3db9dd3..b96cfaadc 100644 --- a/pkg/lightning/mydump/csv_parser.go +++ b/pkg/lightning/mydump/csv_parser.go @@ -38,25 +38,25 @@ var ( type CSVParser struct { blockParser cfg *config.CSVConfig - escFlavor backslashEscapeFlavor - + comma []byte quote []byte quoteIndexFunc func([]byte) int unquoteIndexFunc func([]byte) int - + // recordBuffer holds the unescaped fields, one after another. // The fields can be accessed by using the indexes in fieldIndexes. // E.g., For the row `a,"b","c""d",e`, recordBuffer will contain `abc"de` // and fieldIndexes will contain the indexes [1, 2, 5, 6]. recordBuffer []byte - + // fieldIndexes is an index of fields inside recordBuffer. // The i'th field ends at offset fieldIndexes[i] in recordBuffer. fieldIndexes []int - + lastRecord []string - + + escFlavor backslashEscapeFlavor // if set to true, csv parser will treat the first non-empty line as header line shouldParseHeader bool } diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 3cec18729..f70fd7866 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -147,22 +147,22 @@ type RestoreController struct { tidbGlue glue.Glue postProcessLock sync.Mutex // a simple way to ensure post-processing is not concurrent without using complicated goroutines alterTableLock sync.Mutex - compactState int32 sysVars map[string]string tls *common.TLS - + errorSummaries errorSummaries - + checkpointsDB CheckpointsDB saveCpCh chan saveCp checkpointsWg sync.WaitGroup - + closedEngineLimit *worker.Pool store storage.ExternalStorage checksumManager ChecksumManager - + diskQuotaLock sync.RWMutex diskQuotaState int32 + compactState int32 } func NewRestoreController( From 0404f77e9e5fdcaf8e093ecc0985e276ecf727d3 Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 16:45:05 +0800 Subject: [PATCH 03/29] *: fix lightning gocritic --- pkg/lightning/backend/local.go | 8 +++----- pkg/lightning/backend/sql2kv.go | 11 ++++++----- pkg/lightning/backend/tidb.go | 14 +++++++------- pkg/lightning/lightning.go | 7 ++++++- pkg/lightning/mydump/csv_parser.go | 15 +++++++-------- pkg/lightning/mydump/parser.go | 2 +- pkg/lightning/mydump/reader_test.go | 2 -- pkg/lightning/mydump/region.go | 1 - pkg/lightning/mydump/router_test.go | 2 +- pkg/lightning/restore/restore.go | 26 +++++++++++++------------- 10 files changed, 44 insertions(+), 44 deletions(-) diff --git a/pkg/lightning/backend/local.go b/pkg/lightning/backend/local.go index 84a2d3872..04f1a92d8 100644 --- a/pkg/lightning/backend/local.go +++ b/pkg/lightning/backend/local.go @@ -809,11 +809,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)) } } diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 4c745f490..65d4ead10 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -302,15 +302,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 { 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)) @@ -318,11 +319,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 { diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 4a90f54ed..13951bac3 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -187,13 +187,13 @@ func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, _ *ta 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: @@ -506,11 +506,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. diff --git a/pkg/lightning/lightning.go b/pkg/lightning/lightning.go index ea5ddea30..b963c6723 100755 --- a/pkg/lightning/lightning.go +++ b/pkg/lightning/lightning.go @@ -370,6 +370,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 { @@ -684,7 +689,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/mydump/csv_parser.go b/pkg/lightning/mydump/csv_parser.go index b96cfaadc..d9807c462 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,25 +36,25 @@ var ( // CSVParser is basically a copy of encoding/csv, but special-cased for MySQL-like input. type CSVParser struct { blockParser - cfg *config.CSVConfig - + cfg *config.CSVConfig + comma []byte quote []byte quoteIndexFunc func([]byte) int unquoteIndexFunc func([]byte) int - + // recordBuffer holds the unescaped fields, one after another. // The fields can be accessed by using the indexes in fieldIndexes. // E.g., For the row `a,"b","c""d",e`, recordBuffer will contain `abc"de` // and fieldIndexes will contain the indexes [1, 2, 5, 6]. recordBuffer []byte - + // fieldIndexes is an index of fields inside recordBuffer. // The i'th field ends at offset fieldIndexes[i] in recordBuffer. fieldIndexes []int - + lastRecord []string - + escFlavor backslashEscapeFlavor // if set to true, csv parser will treat the first non-empty line as header line shouldParseHeader bool @@ -290,7 +289,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 } diff --git a/pkg/lightning/mydump/parser.go b/pkg/lightning/mydump/parser.go index e08d3e3ac..e1d6ef097 100644 --- a/pkg/lightning/mydump/parser.go +++ b/pkg/lightning/mydump/parser.go @@ -277,7 +277,7 @@ func unescape( if len(delim) > 0 { delim2 := delim + delim if strings.Index(input, delim2) != -1 { - input = strings.Replace(input, delim2, delim, -1) + input = strings.ReplaceAll(input, delim2, delim) } } if escFlavor != backslashEscapeFlavorNone && strings.IndexByte(input, '\\') != -1 { diff --git a/pkg/lightning/mydump/reader_test.go b/pkg/lightning/mydump/reader_test.go index 7785c86b1..7a85da33f 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{} diff --git a/pkg/lightning/mydump/region.go b/pkg/lightning/mydump/region.go index 9d7f1fb51..ee3440c0f 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -58,7 +58,6 @@ func (reg *TableRegion) Size() int64 { return reg.Chunk.EndOffset - reg.Chunk.Offset } -//////////////////////////////////////////////////////////////// func AllocateEngineIDs( filesRegions []*TableRegion, 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/restore.go b/pkg/lightning/restore/restore.go index f70fd7866..12e53dc5c 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -149,20 +149,20 @@ type RestoreController struct { alterTableLock sync.Mutex sysVars map[string]string tls *common.TLS - + errorSummaries errorSummaries - + checkpointsDB CheckpointsDB saveCpCh chan saveCp checkpointsWg sync.WaitGroup - + closedEngineLimit *worker.Pool store storage.ExternalStorage checksumManager ChecksumManager - + diskQuotaLock sync.RWMutex diskQuotaState int32 - compactState int32 + compactState int32 } func NewRestoreController( @@ -906,16 +906,17 @@ 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" } @@ -1676,11 +1677,12 @@ func (t *TableRestore) postProcess( // 5. do table analyze if cp.Status < CheckpointStatusAnalyzed { - if rc.cfg.PostRestore.Analyze == config.OpLevelOff { + switch { + case 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 { + case forcePostProcess || !rc.cfg.PostRestore.PostProcessAtLast: err := t.analyzeTable(ctx, rc.tidbGlue.GetSQLExecutor()) // witch post restore level 'optional', we will skip analyze error if rc.cfg.PostRestore.Analyze == config.OpLevelOptional { @@ -1694,7 +1696,7 @@ func (t *TableRestore) postProcess( return false, errors.Trace(err) } cp.Status = CheckpointStatusAnalyzed - } else { + default: finished = false } } @@ -2214,8 +2216,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 = 65536 // 64 KB. batch at least this amount of bytes to reduce number of messages From 20c7b3f07b4abf021da57863fc9694bce2c3e9ec Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 17:43:10 +0800 Subject: [PATCH 04/29] lightning: fix stylecheck --- cmd/tidb-lightning-ctl/main.go | 2 +- pkg/lightning/backend/importer_test.go | 33 ++- pkg/lightning/backend/local.go | 46 ++-- pkg/lightning/backend/local_test.go | 8 +- pkg/lightning/backend/localhelper_test.go | 10 +- pkg/lightning/backend/sql2kv.go | 10 +- pkg/lightning/backend/sql2kv_test.go | 4 +- pkg/lightning/backend/tidb.go | 4 +- pkg/lightning/checkpoints/checkpoints.go | 6 +- pkg/lightning/checkpoints/glue_checkpoint.go | 10 +- pkg/lightning/config/config.go | 10 +- pkg/lightning/mydump/csv_parser.go | 4 +- pkg/lightning/mydump/region.go | 5 +- pkg/lightning/restore/checksum.go | 34 +-- pkg/lightning/restore/checksum_test.go | 16 +- pkg/lightning/restore/restore.go | 276 +++++++++---------- pkg/lightning/restore/restore_test.go | 109 ++++---- pkg/lightning/restore/tidb.go | 12 +- 18 files changed, 298 insertions(+), 301 deletions(-) diff --git a/cmd/tidb-lightning-ctl/main.go b/cmd/tidb-lightning-ctl/main.go index 54e04a312..8854f60b5 100644 --- a/cmd/tidb-lightning-ctl/main.go +++ b/cmd/tidb-lightning-ctl/main.go @@ -260,7 +260,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 := kv.MakeUUID(table.TableName, engineID) - file := kv.LocalFile{Uuid: eID} + file := kv.LocalFile{UUID: eID} err := file.Cleanup(cfg.TikvImporter.SortedKVDir) if err != nil { fmt.Fprintln(os.Stderr, "* Encountered error while cleanup engine:", err) diff --git a/pkg/lightning/backend/importer_test.go b/pkg/lightning/backend/importer_test.go index 5819e2da9..eb378ff70 100644 --- a/pkg/lightning/backend/importer_test.go +++ b/pkg/lightning/backend/importer_test.go @@ -22,7 +22,6 @@ import ( "github.com/google/uuid" . "github.com/pingcap/check" "github.com/pingcap/errors" - "github.com/pingcap/kvproto/pkg/import_kvpb" kvpb "github.com/pingcap/kvproto/pkg/import_kvpb" @@ -69,7 +68,7 @@ func (s *importerSuite) setUpTest(c *C) { }) s.mockClient.EXPECT(). - OpenEngine(s.ctx, &import_kvpb.OpenEngineRequest{Uuid: s.engineUUID}). + OpenEngine(s.ctx, &kvpb.OpenEngineRequest{Uuid: s.engineUUID}). Return(nil, nil) var err error @@ -88,18 +87,18 @@ func (s *importerSuite) TestWriteRows(c *C) { s.mockClient.EXPECT().WriteEngine(s.ctx).Return(s.mockWriter, nil) headSendCall := s.mockWriter.EXPECT(). - Send(&import_kvpb.WriteEngineRequest{ - Chunk: &import_kvpb.WriteEngineRequest_Head{ - Head: &import_kvpb.WriteHead{Uuid: s.engineUUID}, + Send(&kvpb.WriteEngineRequest{ + Chunk: &kvpb.WriteEngineRequest_Head{ + Head: &kvpb.WriteHead{Uuid: s.engineUUID}, }, }). Return(nil) batchSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { - c.Assert(x.GetBatch().GetMutations(), DeepEquals, []*import_kvpb.Mutation{ - {Op: import_kvpb.Mutation_Put, Key: []byte("k1"), Value: []byte("v1")}, - {Op: import_kvpb.Mutation_Put, Key: []byte("k2"), Value: []byte("v2")}, + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { + c.Assert(x.GetBatch().GetMutations(), DeepEquals, []*kvpb.Mutation{ + {Op: kvpb.Mutation_Put, Key: []byte("k1"), Value: []byte("v1")}, + {Op: kvpb.Mutation_Put, Key: []byte("k2"), Value: []byte("v2")}, }) return nil }). @@ -121,7 +120,7 @@ func (s *importerSuite) TestWriteHeadSendFailed(c *C) { headSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { c.Assert(x.GetHead(), NotNil) return errors.Annotate(context.Canceled, "fake unrecoverable write head error") }) @@ -142,13 +141,13 @@ func (s *importerSuite) TestWriteBatchSendFailed(c *C) { headSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { c.Assert(x.GetHead(), NotNil) return nil }) batchSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { c.Assert(x.GetBatch(), NotNil) return errors.Annotate(context.Canceled, "fake unrecoverable write batch error") }). @@ -170,13 +169,13 @@ func (s *importerSuite) TestWriteCloseFailed(c *C) { headSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { c.Assert(x.GetHead(), NotNil) return nil }) batchSendCall := s.mockWriter.EXPECT(). Send(gomock.Any()). - DoAndReturn(func(x *import_kvpb.WriteEngineRequest) error { + DoAndReturn(func(x *kvpb.WriteEngineRequest) error { c.Assert(x.GetBatch(), NotNil) return nil }). @@ -195,13 +194,13 @@ func (s *importerSuite) TestCloseImportCleanupEngine(c *C) { defer s.tearDownTest() s.mockClient.EXPECT(). - CloseEngine(s.ctx, &import_kvpb.CloseEngineRequest{Uuid: s.engineUUID}). + CloseEngine(s.ctx, &kvpb.CloseEngineRequest{Uuid: s.engineUUID}). Return(nil, nil) s.mockClient.EXPECT(). - ImportEngine(s.ctx, &import_kvpb.ImportEngineRequest{Uuid: s.engineUUID, PdAddr: testPDAddr}). + ImportEngine(s.ctx, &kvpb.ImportEngineRequest{Uuid: s.engineUUID, PdAddr: testPDAddr}). Return(nil, nil) s.mockClient.EXPECT(). - CleanupEngine(s.ctx, &import_kvpb.CleanupEngineRequest{Uuid: s.engineUUID}). + CleanupEngine(s.ctx, &kvpb.CleanupEngineRequest{Uuid: s.engineUUID}). Return(nil, nil) engine, err := s.engine.Close(s.ctx) diff --git a/pkg/lightning/backend/local.go b/pkg/lightning/backend/local.go index 04f1a92d8..17a887629 100644 --- a/pkg/lightning/backend/local.go +++ b/pkg/lightning/backend/local.go @@ -107,7 +107,7 @@ type Range struct { // 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. @@ -126,7 +126,7 @@ const ( type LocalFile struct { localFileMeta db *pebble.DB - Uuid uuid.UUID + UUID uuid.UUID localWriters sync.Map // isImportingAtomic is an atomic variable indicating whether the importMutex has been locked. @@ -136,7 +136,7 @@ type LocalFile struct { } func (e *LocalFile) 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 } @@ -147,13 +147,13 @@ func (e *LocalFile) Close() error { // Cleanup remove meta and db files func (e *LocalFile) 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 *LocalFile) 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 } @@ -163,7 +163,7 @@ func (e *LocalFile) Exist(dataDir string) error { func (e *LocalFile) 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) } @@ -174,7 +174,7 @@ func (e *LocalFile) 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) } @@ -205,7 +205,7 @@ func (e *LocalFile) getEngineFileSize() EngineFileSize { }) return EngineFileSize{ - UUID: e.Uuid, + UUID: e.UUID, DiskSize: total.Size, MemSize: memSize, IsImporting: e.isLocked(), @@ -295,14 +295,14 @@ func (e *LocalFile) saveEngineMeta() error { func (e *LocalFile) 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)) } } @@ -459,8 +459,8 @@ func NewLocalBackend( } // lock locks a local file and returns the LocalFile instance if it exists. -func (local *local) lockEngine(engineId uuid.UUID, state importMutexState) *LocalFile { - if e, ok := local.engines.Load(engineId); ok { +func (local *local) lockEngine(engineID uuid.UUID, state importMutexState) *LocalFile { + if e, ok := local.engines.Load(engineID); ok { engine := e.(*LocalFile) engine.lock(state) return engine @@ -549,13 +549,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) @@ -619,7 +619,7 @@ func (local *local) OpenEngine(ctx context.Context, engineUUID uuid.UUID) error if err != nil { return err } - e, _ := local.engines.LoadOrStore(engineUUID, &LocalFile{Uuid: engineUUID}) + e, _ := local.engines.LoadOrStore(engineUUID, &LocalFile{UUID: engineUUID}) engine := e.(*LocalFile) engine.db = db engine.loadEngineMeta() @@ -644,7 +644,7 @@ func (local *local) CloseEngine(ctx context.Context, engineUUID uuid.UUID) error return err } engineFile := &LocalFile{ - Uuid: engineUUID, + UUID: engineUUID, db: db, } engineFile.loadEngineMeta() @@ -744,7 +744,7 @@ func (local *local) WriteToTiKV( } req.Chunk = &sst.WriteRequest_Batch{ Batch: &sst.WriteBatch{ - CommitTs: engineFile.Ts, + CommitTs: engineFile.TS, }, } clients = append(clients, wstream) @@ -939,7 +939,7 @@ func (local *local) readAndSplitIntoRange(engineFile *LocalFile) ([]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))) @@ -1239,7 +1239,7 @@ loopWrite: func (local *local) writeAndIngestByRanges(ctx context.Context, engineFile *LocalFile, 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))) @@ -1568,8 +1568,8 @@ func nextKey(key []byte) []byte { // in tikv <= 4.x, tikv will truncate the row key, so we should fetch the next valid row key // See: https://github.com/tikv/tikv/blob/f7f22f70e1585d7ca38a59ea30e774949160c3e8/components/raftstore/src/coprocessor/split_observer.rs#L36-L41 if tablecodec.IsRecordKey(key) { - tableId, handle, _ := tablecodec.DecodeRecordKey(key) - return tablecodec.EncodeRowKeyWithHandle(tableId, handle.Next()) + tableID, handle, _ := tablecodec.DecodeRecordKey(key) + return tablecodec.EncodeRowKeyWithHandle(tableID, handle.Next()) } // if key is an index, directly append a 0x00 to the key. @@ -1775,7 +1775,7 @@ func (w *LocalWriter) AppendRows(ctx context.Context, tableName string, columnNa return err } w.kvsChan <- kvs - w.local.Ts = ts + w.local.TS = ts return nil } diff --git a/pkg/lightning/backend/local_test.go b/pkg/lightning/backend/local_test.go index 12f68bbe3..833c34d22 100644 --- a/pkg/lightning/backend/local_test.go +++ b/pkg/lightning/backend/local_test.go @@ -66,9 +66,9 @@ func (s *localSuite) TestNextKey(c *C) { // test recode key // key with int handle - for _, handleId := range []int64{1, 255, math.MaxInt32} { - key := tablecodec.EncodeRowKeyWithHandle(1, kv.IntHandle(handleId)) - c.Assert(nextKey(key), DeepEquals, []byte(tablecodec.EncodeRowKeyWithHandle(1, kv.IntHandle(handleId+1)))) + for _, handleID := range []int64{1, 255, math.MaxInt32} { + key := tablecodec.EncodeRowKeyWithHandle(1, kv.IntHandle(handleID)) + c.Assert(nextKey(key), DeepEquals, []byte(tablecodec.EncodeRowKeyWithHandle(1, kv.IntHandle(handleID+1)))) } testDatums := [][]types.Datum{ @@ -312,7 +312,7 @@ func testLocalWriter(c *C, needSort bool, partitialSort bool) { c.Assert(err, IsNil) meta := localFileMeta{} _, engineUUID := MakeUUID("ww", 0) - f := LocalFile{localFileMeta: meta, db: db, Uuid: engineUUID} + f := LocalFile{localFileMeta: meta, db: db, UUID: engineUUID} w := openLocalWriter(&f, tmpPath, 1024*1024) ctx := context.Background() diff --git a/pkg/lightning/backend/localhelper_test.go b/pkg/lightning/backend/localhelper_test.go index 019696c04..8920b88f2 100644 --- a/pkg/lightning/backend/localhelper_test.go +++ b/pkg/lightning/backend/localhelper_test.go @@ -470,9 +470,9 @@ func (s *localSuite) doTestBatchSplitByRangesWithClusteredIndex(c *C, hook clien stmtCtx := new(stmtctx.StatementContext) - tableId := int64(1) - tableStartKey := tablecodec.EncodeTablePrefix(tableId) - tableEndKey := tablecodec.EncodeTablePrefix(tableId + 1) + tableID := int64(1) + tableStartKey := tablecodec.EncodeTablePrefix(tableID) + tableEndKey := tablecodec.EncodeTablePrefix(tableID + 1) keys := [][]byte{[]byte(""), tableStartKey} // pre split 2 regions for i := int64(0); i < 2; i++ { @@ -480,7 +480,7 @@ func (s *localSuite) doTestBatchSplitByRangesWithClusteredIndex(c *C, hook clien c.Assert(err, IsNil) h, err := kv.NewCommonHandle(keyBytes) c.Assert(err, IsNil) - key := tablecodec.EncodeRowKeyWithHandle(tableId, h) + key := tablecodec.EncodeRowKeyWithHandle(tableID, h) keys = append(keys, key) } keys = append(keys, tableEndKey, []byte("")) @@ -498,7 +498,7 @@ func (s *localSuite) doTestBatchSplitByRangesWithClusteredIndex(c *C, hook clien c.Assert(err, IsNil) h, err := kv.NewCommonHandle(keyBytes) c.Assert(err, IsNil) - key := tablecodec.EncodeRowKeyWithHandle(tableId, h) + key := tablecodec.EncodeRowKeyWithHandle(tableID, h) rangeKeys = append(rangeKeys, key) } } diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 65d4ead10..2628b5ac4 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -409,8 +409,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 } @@ -418,17 +418,17 @@ 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, kvPairs(kvs[i:j])) i = j cumSize = 0 } cumSize += size } - return append(res, kvPairs(totalKVs[i:])) + return append(res, kvPairs(kvs[i:])) } func (kvs kvPairs) Clear() Rows { diff --git a/pkg/lightning/backend/sql2kv_test.go b/pkg/lightning/backend/sql2kv_test.go index 1ee96921d..b41299033 100644 --- a/pkg/lightning/backend/sql2kv_test.go +++ b/pkg/lightning/backend/sql2kv_test.go @@ -215,9 +215,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) diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 13951bac3..6b0bcc860 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -49,8 +49,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 diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index 5d0a9935f..33dfa8872 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -446,7 +446,7 @@ type DestroyedTableCheckpoint struct { } type TaskCheckpoint struct { - TaskId int64 + TaskID int64 SourceDir string Backend string ImporterAddr string @@ -675,7 +675,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 @@ -1000,7 +1000,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, diff --git a/pkg/lightning/checkpoints/glue_checkpoint.go b/pkg/lightning/checkpoints/glue_checkpoint.go index ef9edf7e5..a5f70cc53 100644 --- a/pkg/lightning/checkpoints/glue_checkpoint.go +++ b/pkg/lightning/checkpoints/glue_checkpoint.go @@ -195,7 +195,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) @@ -580,10 +580,10 @@ func (g GlueCheckpointsDB) GetLocalStoringTables(ctx context.Context) (map[strin // 2. engine status is earlier than CheckpointStatusImported, and // 3. chunk has been read query := fmt.Sprintf(` - SELECT DISTINCT t.table_name, c.engine_id - FROM %s.%s t, %s.%s c, %s.%s e - WHERE t.table_name = c.table_name AND t.table_name = e.table_name AND c.engine_id = e.engine_id - AND %d < t.status AND t.status < %d + SELECT DISTINCT t.table_name, c.engine_id + FROM %s.%s t, %s.%s c, %s.%s e + WHERE t.table_name = c.table_name AND t.table_name = e.table_name AND c.engine_id = e.engine_id + AND %d < t.status AND t.status < %d AND %d < e.status AND e.status < %d AND c.pos > c.offset;`, g.schema, CheckpointTableNameTable, g.schema, CheckpointTableNameChunk, g.schema, CheckpointTableNameEngine, diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index 3dc2f4ea9..1b2c39ce9 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -146,17 +146,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 { diff --git a/pkg/lightning/mydump/csv_parser.go b/pkg/lightning/mydump/csv_parser.go index d9807c462..9d1d7258f 100644 --- a/pkg/lightning/mydump/csv_parser.go +++ b/pkg/lightning/mydump/csv_parser.go @@ -566,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/region.go b/pkg/lightning/mydump/region.go index ee3440c0f..511944392 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -58,7 +58,6 @@ func (reg *TableRegion) Size() int64 { return reg.Chunk.EndOffset - reg.Chunk.Offset } - func AllocateEngineIDs( filesRegions []*TableRegion, dataFileSizes []float64, @@ -332,7 +331,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 @@ -359,7 +358,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/restore/checksum.go b/pkg/lightning/restore/checksum.go index 508a28bf6..58808f2e7 100644 --- a/pkg/lightning/restore/checksum.go +++ b/pkg/lightning/restore/checksum.go @@ -22,7 +22,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,7 +52,7 @@ 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) { @@ -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 { @@ -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) { executor, err := checksum.NewExecutorBuilder(tableInfo.Core, oracle.ComposeTS(time.Now().Unix()*1000, 0)). SetConcurrency(e.distSQLScanConcurrency). Build() @@ -308,7 +308,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 { @@ -352,7 +352,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 @@ -372,15 +372,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 } @@ -405,18 +405,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 02cfdfb39..6078ed527 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) } } @@ -307,23 +307,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/restore.go b/pkg/lightning/restore/restore.go index 12e53dc5c..804bcc919 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -37,7 +37,7 @@ import ( "modernc.org/mathutil" kv "github.com/pingcap/br/pkg/lightning/backend" - . "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" @@ -82,11 +82,11 @@ func init() { type saveCp struct { tableName string - merger TableCheckpointMerger + merger checkpoints.TableCheckpointMerger } type errorSummary struct { - status CheckpointStatus + status checkpoints.CheckpointStatus err error } @@ -121,7 +121,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} @@ -136,7 +136,7 @@ const ( type RestoreController struct { cfg *config.Config dbMetas []*mydump.MDDatabaseMeta - dbInfos map[string]*TidbDBInfo + dbInfos map[string]*checkpoints.TidbDBInfo tableWorkers *worker.Pool indexWorkers *worker.Pool regionWorkers *worker.Pool @@ -152,7 +152,7 @@ type RestoreController struct { errorSummaries errorSummaries - checkpointsDB CheckpointsDB + checkpointsDB checkpoints.CheckpointsDB saveCpCh chan saveCp checkpointsWg sync.WaitGroup @@ -451,7 +451,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() @@ -601,7 +601,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 } @@ -657,7 +657,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.CheckpointsDB, dir string) error { targetTables, err := cpdb.GetLocalStoringTables(ctx) if err != nil { return errors.Trace(err) @@ -665,7 +665,7 @@ func verifyLocalFile(ctx context.Context, cpdb CheckpointsDB, dir string) error for tableName, engineIDs := range targetTables { for _, engineID := range engineIDs { _, eID := kv.MakeUUID(tableName, engineID) - file := kv.LocalFile{Uuid: eID} + file := kv.LocalFile{UUID: eID} err := file.Exist(dir) if err != nil { log.L().Error("can't find local file", @@ -691,11 +691,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 { @@ -735,8 +735,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 *RestoreController) 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)) @@ -751,7 +751,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) @@ -765,7 +765,7 @@ func (rc *RestoreController) 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) @@ -774,7 +774,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 { @@ -789,7 +789,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) @@ -802,7 +802,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { lock.Unlock() 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") @@ -810,7 +810,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { }) 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") @@ -818,9 +818,9 @@ func (rc *RestoreController) listenCheckpointUpdates() { }) 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") @@ -828,7 +828,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { }) failpoint.Inject("KillIfImportedChunk", 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)) { common.KillMySelf() } }) @@ -1010,7 +1010,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { type task struct { tr *TableRestore - cp *TableCheckpoint + cp *checkpoints.TableCheckpoint } totalTables := 0 @@ -1054,7 +1054,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 { @@ -1073,7 +1073,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{}{} @@ -1093,7 +1093,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") @@ -1190,10 +1190,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, + cp *checkpoints.TableCheckpoint, ) (bool, error) { // 1. Load the table info. @@ -1205,49 +1205,49 @@ 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) + tr.alloc.Get(autoid.AutoRandomType).Rebase(tr.tableInfo.ID, cp.AllocBase, false) } 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) + tr.alloc.Get(autoid.RowIDAllocType).Rebase(tr.tableInfo.ID, cp.AllocBase, false) } 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(ctx context.Context, rc *RestoreController, cp *TableCheckpoint) error { +func (tr *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController, 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) } // The table checkpoint status set to `CheckpointStatusIndexImported` only if @@ -1258,11 +1258,11 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController // when kill lightning after saving index engine checkpoint status before saving // table checkpoint status. var closedIndexEngine *kv.ClosedEngine - if indexEngineCp.Status < CheckpointStatusImported && cp.Status < CheckpointStatusIndexImported { + if indexEngineCp.Status < checkpoints.CheckpointStatusImported && cp.Status < checkpoints.CheckpointStatusIndexImported { indexWorker := rc.indexWorkers.Apply() defer rc.indexWorkers.Recycle(indexWorker) - indexEngine, err := rc.backend.OpenEngine(ctx, t.tableName, indexEngineID) + indexEngine, err := rc.backend.OpenEngine(ctx, tr.tableName, indexEngineID) if err != nil { return errors.Trace(err) } @@ -1275,7 +1275,7 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController cp.Status, indexEngineCp.Status) } - 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 @@ -1297,7 +1297,7 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController continue } - if engine.Status < CheckpointStatusImported { + if engine.Status < checkpoints.CheckpointStatusImported { wg.Add(1) // Note: We still need tableWorkers to control the concurrency of tables. @@ -1305,11 +1305,11 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController // 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 { @@ -1323,7 +1323,7 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController 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 { engineErr.Set(err) } }(restoreWorker, engineID, engine) @@ -1340,26 +1340,26 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController // If index engine file has been closed but not imported only if context cancel occurred // when `importKV()` execution, so `UnsafeCloseEngine` and continue import it. - if indexEngineCp.Status == CheckpointStatusClosed { - closedIndexEngine, err = rc.backend.UnsafeCloseEngine(ctx, t.tableName, indexEngineID) + if indexEngineCp.Status == checkpoints.CheckpointStatusClosed { + closedIndexEngine, err = rc.backend.UnsafeCloseEngine(ctx, tr.tableName, indexEngineID) } else { closedIndexEngine, err = indexEngine.Close(ctx) - rc.saveStatusCheckpoint(t.tableName, indexEngineID, err, CheckpointStatusClosed) + rc.saveStatusCheckpoint(tr.tableName, indexEngineID, err, checkpoints.CheckpointStatusClosed) } if err != nil { return errors.Trace(err) } } - if cp.Status < CheckpointStatusIndexImported { + if cp.Status < checkpoints.CheckpointStatusIndexImported { var err error - if indexEngineCp.Status < CheckpointStatusImported { + if indexEngineCp.Status < checkpoints.CheckpointStatusImported { // the lock ensures the import() step will not be concurrent. if !rc.isLocalBackend() { rc.postProcessLock.Lock() } - err = t.importKV(ctx, closedIndexEngine, rc, indexEngineID) - rc.saveStatusCheckpoint(t.tableName, indexEngineID, err, CheckpointStatusImported) + err = tr.importKV(ctx, closedIndexEngine, rc, indexEngineID) + rc.saveStatusCheckpoint(tr.tableName, indexEngineID, err, checkpoints.CheckpointStatusImported) if !rc.isLocalBackend() { rc.postProcessLock.Unlock() } @@ -1369,7 +1369,7 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController 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) } @@ -1377,15 +1377,15 @@ func (t *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController return nil } -func (t *TableRestore) restoreEngine( +func (tr *TableRestore) restoreEngine( ctx context.Context, rc *RestoreController, indexEngine *kv.OpenedEngine, engineID int32, - cp *EngineCheckpoint, + cp *checkpoints.EngineCheckpoint, ) (*kv.ClosedEngine, error) { - if cp.Status >= CheckpointStatusClosed { - closedEngine, err := rc.backend.UnsafeCloseEngine(ctx, t.tableName, engineID) + if cp.Status >= checkpoints.CheckpointStatusClosed { + closedEngine, err := rc.backend.UnsafeCloseEngine(ctx, tr.tableName, engineID) // If any error occurred, recycle worker immediately if err != nil { return closedEngine, errors.Trace(err) @@ -1410,9 +1410,9 @@ func (t *TableRestore) restoreEngine( return nil, errors.Trace(err) } - 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) + dataEngine, err := rc.backend.OpenEngine(ctx, tr.tableName, engineID) if err != nil { return nil, errors.Trace(err) } @@ -1441,7 +1441,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) } @@ -1465,7 +1465,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() } @@ -1504,7 +1504,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 } @@ -1513,7 +1513,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 @@ -1540,10 +1540,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) @@ -1551,14 +1551,14 @@ func (t *TableRestore) restoreEngine( return closedDataEngine, nil } -func (t *TableRestore) importEngine( +func (tr *TableRestore) importEngine( ctx context.Context, closedEngine *kv.ClosedEngine, rc *RestoreController, engineID int32, - cp *EngineCheckpoint, + cp *checkpoints.EngineCheckpoint, ) error { - if cp.Status >= CheckpointStatusImported { + if cp.Status >= checkpoints.CheckpointStatusImported { return nil } @@ -1569,8 +1569,8 @@ func (t *TableRestore) importEngine( if !rc.isLocalBackend() { rc.postProcessLock.Lock() } - err := t.importKV(ctx, closedEngine, rc, engineID) - rc.saveStatusCheckpoint(t.tableName, engineID, err, CheckpointStatusImported) + err := tr.importKV(ctx, closedEngine, rc, engineID) + rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusImported) if !rc.isLocalBackend() { rc.postProcessLock.Unlock() } @@ -1596,10 +1596,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, + cp *checkpoints.TableCheckpoint, forcePostProcess bool, ) (bool, error) { // there are no data in this table, no need to do post process @@ -1610,28 +1610,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 } @@ -1639,10 +1639,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 @@ -1652,20 +1652,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 } @@ -1676,26 +1676,26 @@ func (t *TableRestore) postProcess( } // 5. do table analyze - if cp.Status < CheckpointStatusAnalyzed { + if cp.Status < checkpoints.CheckpointStatusAnalyzed { switch { case rc.cfg.PostRestore.Analyze == config.OpLevelOff: - t.logger.Info("skip analyze") - rc.saveStatusCheckpoint(t.tableName, WholeTableEngineID, nil, CheckpointStatusAnalyzeSkipped) - cp.Status = CheckpointStatusAnalyzed + 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 := t.analyzeTable(ctx, rc.tidbGlue.GetSQLExecutor()) + 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 + cp.Status = checkpoints.CheckpointStatusAnalyzed default: finished = false } @@ -1904,17 +1904,17 @@ func (rc *RestoreController) isLocalBackend() bool { 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) @@ -1966,8 +1966,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 @@ -1977,9 +1977,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) @@ -2003,9 +2003,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 *RestoreController, 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) { @@ -2014,13 +2014,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, }, @@ -2030,7 +2030,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) } @@ -2040,7 +2040,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)), @@ -2062,14 +2062,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 { @@ -2077,7 +2077,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) } @@ -2087,8 +2087,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 { @@ -2096,7 +2096,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 } @@ -2111,12 +2111,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), ) @@ -2126,7 +2126,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) } @@ -2170,7 +2170,7 @@ func (tr *TableRestore) importKV( 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) } @@ -2344,7 +2344,7 @@ func (cr *chunkRestore) deliverLoop( return } -func saveCheckpoint(rc *RestoreController, t *TableRestore, engineID int32, chunk *ChunkCheckpoint) { +func saveCheckpoint(rc *RestoreController, 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. @@ -2357,13 +2357,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, diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index c32760827..3f7eefdbd 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -37,7 +37,6 @@ import ( kv "github.com/pingcap/br/pkg/lightning/backend" "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" @@ -66,7 +65,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) @@ -74,7 +73,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, } @@ -83,23 +82,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.*`) } @@ -107,8 +106,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() @@ -207,8 +206,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 @@ -237,10 +236,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 @@ -281,7 +280,7 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { 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() @@ -293,22 +292,22 @@ 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") - 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} err := s.tr.populateChunks(context.Background(), rc, cp) c.Assert(err, 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: 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, @@ -319,7 +318,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, @@ -330,7 +329,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, @@ -343,10 +342,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, @@ -357,7 +356,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, @@ -368,7 +367,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, @@ -381,10 +380,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, @@ -451,8 +450,8 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(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") - cp := &TableCheckpoint{ - Engines: make(map[int32]*EngineCheckpoint), + cp := &checkpoints.TableCheckpoint{ + Engines: make(map[int32]*checkpoints.EngineCheckpoint), } cfg := config.NewConfig() @@ -463,19 +462,19 @@ func (s *tableRestoreSuite) TestPopulateChunksCSVHeader(c *C) { cfg.Mydumper.StrictFormat = true rc := &RestoreController{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, @@ -486,7 +485,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, @@ -497,7 +496,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{ @@ -511,7 +510,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{ @@ -524,7 +523,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{ @@ -539,10 +538,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{ @@ -555,7 +554,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{ @@ -568,7 +567,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{ @@ -583,10 +582,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{ @@ -618,7 +617,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}) @@ -792,8 +791,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, diff --git a/pkg/lightning/restore/tidb.go b/pkg/lightning/restore/tidb.go index a672e7742..8f77c29b6 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, From 4e8b7c7f28e469a1c51bee4121c56a3e66cb28bf Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 18:10:14 +0800 Subject: [PATCH 05/29] lightning: fix golint --- Makefile | 2 +- cmd/tidb-lightning/main.go | 11 ++- pkg/lightning/backend/local.go | 4 +- pkg/lightning/backend/session.go | 6 -- pkg/lightning/backend/sql2kv.go | 2 +- pkg/lightning/checkpoints/checkpoints.go | 22 +++--- pkg/lightning/checkpoints/glue_checkpoint.go | 2 +- pkg/lightning/config/configlist.go | 24 +++--- pkg/lightning/glue/glue.go | 4 +- pkg/lightning/lightning.go | 4 +- pkg/lightning/mock/glue.go | 8 +- pkg/lightning/mydump/csv_parser.go | 3 +- pkg/lightning/mydump/loader.go | 40 +++++----- pkg/lightning/mydump/reader_test.go | 4 +- pkg/lightning/mydump/region.go | 1 - pkg/lightning/mydump/router.go | 1 - pkg/lightning/restore/checksum.go | 6 +- pkg/lightning/restore/restore.go | 82 ++++++++++---------- pkg/lightning/restore/restore_test.go | 30 +++---- 19 files changed, 121 insertions(+), 135 deletions(-) diff --git a/Makefile b/Makefile index 16c928d16..9e3187a69 100644 --- a/Makefile +++ b/Makefile @@ -208,7 +208,7 @@ static: prepare tools --disable exhaustive \ --disable godot \ --disable gosec \ - $$($(PACKAGE_DIRECTORIES) | grep -v "lightning") + $$($(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/main.go b/cmd/tidb-lightning/main.go index f577e1109..34f1aa242 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, nil) } + cfg := config.NewConfig() + if err := cfg.LoadFromGlobal(globalCfg); err != nil { + return err + } + return app.RunOnce(context.Background(), cfg, nil, nil) }() if err != nil { diff --git a/pkg/lightning/backend/local.go b/pkg/lightning/backend/local.go index 17a887629..0d07e8d9f 100644 --- a/pkg/lightning/backend/local.go +++ b/pkg/lightning/backend/local.go @@ -996,7 +996,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() @@ -1682,7 +1682,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) diff --git a/pkg/lightning/backend/session.go b/pkg/lightning/backend/session.go index c3c847f01..8088440ed 100644 --- a/pkg/lightning/backend/session.go +++ b/pkg/lightning/backend/session.go @@ -113,12 +113,6 @@ type transaction struct { kvUnionStore } -func NewTransaction() *transaction { - return &transaction{ - kvUnionStore: kvUnionStore{}, - } -} - func (t *transaction) GetMemBuffer() kv.MemBuffer { return &t.kvUnionStore.kvMemBuf } diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 2628b5ac4..7cc1ad54c 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -95,7 +95,7 @@ func autoRandomIncrementBits(col *table.Column, randomBits int) int { incrementalBits := typeBitsLength - randomBits hasSignBit := !mysql.HasUnsignedFlag(col.Flag) if hasSignBit { - incrementalBits -= 1 + incrementalBits-- } return incrementalBits } diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index 33dfa8872..c9ec53961 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -457,7 +457,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) @@ -482,7 +482,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 } @@ -1112,7 +1112,6 @@ func (cpdb *FileCheckpointsDB) InsertEngineCheckpoints(_ context.Context, tableN if len(value.ColumnPermutation) > 0 { chunk.ColumnPermutation = intSlice2Int32Slice(value.ColumnPermutation) } - } tableModel.Engines[engineID] = engineModel } @@ -1157,14 +1156,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) { @@ -1172,23 +1171,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 { @@ -1516,7 +1515,6 @@ func (cpdb *FileCheckpointsDB) GetLocalStoringTables(_ context.Context) (map[str break } } - } } diff --git a/pkg/lightning/checkpoints/glue_checkpoint.go b/pkg/lightning/checkpoints/glue_checkpoint.go index a5f70cc53..da987d2e0 100644 --- a/pkg/lightning/checkpoints/glue_checkpoint.go +++ b/pkg/lightning/checkpoints/glue_checkpoint.go @@ -54,7 +54,7 @@ type GlueCheckpointsDB struct { schema string } -var _ CheckpointsDB = (*GlueCheckpointsDB)(nil) +var _ DB = (*GlueCheckpointsDB)(nil) func NewGlueCheckpointsDB(ctx context.Context, se Session, f func() (Session, error), schemaName string) (*GlueCheckpointsDB, error) { var escapedSchemaName strings.Builder 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 b963c6723..64d639c4c 100755 --- a/pkg/lightning/lightning.go +++ b/pkg/lightning/lightning.go @@ -54,7 +54,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 @@ -301,7 +301,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)) diff --git a/pkg/lightning/mock/glue.go b/pkg/lightning/mock/glue.go index 63ece855e..0505353d3 100644 --- a/pkg/lightning/mock/glue.go +++ b/pkg/lightning/mock/glue.go @@ -8,12 +8,12 @@ import ( context "context" sql "database/sql" gomock "github.com/golang/mock/gomock" - parser "github.com/pingcap/parser" - model "github.com/pingcap/parser/model" checkpoints "github.com/pingcap/br/pkg/lightning/checkpoints" config "github.com/pingcap/br/pkg/lightning/config" glue "github.com/pingcap/br/pkg/lightning/glue" log "github.com/pingcap/br/pkg/lightning/log" + parser "github.com/pingcap/parser" + model "github.com/pingcap/parser/model" reflect "reflect" ) @@ -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 } diff --git a/pkg/lightning/mydump/csv_parser.go b/pkg/lightning/mydump/csv_parser.go index 9d1d7258f..b7f2e6e7a 100644 --- a/pkg/lightning/mydump/csv_parser.go +++ b/pkg/lightning/mydump/csv_parser.go @@ -390,9 +390,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)) diff --git a/pkg/lightning/mydump/loader.go b/pkg/lightning/mydump/loader.go index 182e892be..6728b41e6 100644 --- a/pkg/lightning/mydump/loader.go +++ b/pkg/lightning/mydump/loader.go @@ -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 { @@ -419,16 +419,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 +435,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/reader_test.go b/pkg/lightning/mydump/reader_test.go index 7a85da33f..03bc37024 100644 --- a/pkg/lightning/mydump/reader_test.go +++ b/pkg/lightning/mydump/reader_test.go @@ -167,11 +167,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 { diff --git a/pkg/lightning/mydump/region.go b/pkg/lightning/mydump/region.go index 511944392..7fc06d323 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -260,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 } 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/restore/checksum.go b/pkg/lightning/restore/checksum.go index 58808f2e7..a28f8f50c 100644 --- a/pkg/lightning/restore/checksum.go +++ b/pkg/lightning/restore/checksum.go @@ -55,7 +55,7 @@ type ChecksumManager interface { 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 @@ -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 { diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 804bcc919..c7dc40649 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -133,7 +133,7 @@ const ( diskQuotaStateImporting ) -type RestoreController struct { +type Controller struct { cfg *config.Config dbMetas []*mydump.MDDatabaseMeta dbInfos map[string]*checkpoints.TidbDBInfo @@ -152,7 +152,7 @@ type RestoreController struct { errorSummaries errorSummaries - checkpointsDB checkpoints.CheckpointsDB + checkpointsDB checkpoints.DB saveCpCh chan saveCp checkpointsWg sync.WaitGroup @@ -171,7 +171,7 @@ func NewRestoreController( cfg *config.Config, s storage.ExternalStorage, g glue.Glue, -) (*RestoreController, error) { +) (*Controller, error) { return NewRestoreControllerWithPauser(ctx, dbMetas, cfg, s, DeliverPauser, g) } @@ -182,7 +182,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 @@ -241,7 +241,7 @@ func NewRestoreControllerWithPauser( return nil, errors.New("unknown backend: " + cfg.TikvImporter.Backend) } - rc := &RestoreController{ + rc := &Controller{ cfg: cfg, dbMetas: dbMetas, tableWorkers: worker.NewPool(ctx, cfg.App.TableConcurrency, "table"), @@ -266,12 +266,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, @@ -549,7 +549,7 @@ func (worker *restoreSchemaWorker) appendJob(job *schemaJob) error { } } -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) @@ -657,7 +657,7 @@ func verifyCheckpoint(cfg *config.Config, taskCp *checkpoints.TaskCheckpoint) er } // for local backend, we should check if local SST exists in disk, otherwise we'll lost data -func verifyLocalFile(ctx context.Context, cpdb checkpoints.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) @@ -678,7 +678,7 @@ func verifyLocalFile(ctx context.Context, cpdb checkpoints.CheckpointsDB, dir st 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) @@ -720,10 +720,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++ } } } @@ -735,7 +735,7 @@ func (rc *RestoreController) estimateChunkCountIntoMetrics(ctx context.Context) return nil } -func (rc *RestoreController) saveStatusCheckpoint(tableName string, engineID int32, err error, statusIfSucceed checkpoints.CheckpointStatus) { +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), @@ -761,7 +761,7 @@ 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 @@ -836,7 +836,7 @@ func (rc *RestoreController) listenCheckpointUpdates() { 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 @@ -979,7 +979,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 @@ -1192,7 +1192,7 @@ func (rc *RestoreController) restoreTables(ctx context.Context) error { func (tr *TableRestore) restoreTable( ctx context.Context, - rc *RestoreController, + rc *Controller, cp *checkpoints.TableCheckpoint, ) (bool, error) { // 1. Load the table info. @@ -1244,7 +1244,7 @@ func (tr *TableRestore) restoreTable( return tr.postProcess(ctx, rc, cp, false /* force-analyze */) } -func (tr *TableRestore) restoreEngines(ctx context.Context, rc *RestoreController, cp *checkpoints.TableCheckpoint) error { +func (tr *TableRestore) restoreEngines(ctx 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", tr.tableName) @@ -1271,7 +1271,7 @@ func (tr *TableRestore) restoreEngines(ctx context.Context, rc *RestoreControlle // that index engine checkpoint status less than `CheckpointStatusImported`. // So the index engine must be found in above process if indexEngine == nil { - return errors.Errorf("table checkpoint status %v incompitable with index engine checkpoint status %v", + return errors.Errorf("table checkpoint status %v incompatible with index engine checkpoint status %v", cp.Status, indexEngineCp.Status) } @@ -1379,7 +1379,7 @@ func (tr *TableRestore) restoreEngines(ctx context.Context, rc *RestoreControlle func (tr *TableRestore) restoreEngine( ctx context.Context, - rc *RestoreController, + rc *Controller, indexEngine *kv.OpenedEngine, engineID int32, cp *checkpoints.EngineCheckpoint, @@ -1554,7 +1554,7 @@ func (tr *TableRestore) restoreEngine( func (tr *TableRestore) importEngine( ctx context.Context, closedEngine *kv.ClosedEngine, - rc *RestoreController, + rc *Controller, engineID int32, cp *checkpoints.EngineCheckpoint, ) error { @@ -1598,7 +1598,7 @@ func (tr *TableRestore) importEngine( // post-process-at-last config is true. And if this two phases are skipped, the first return value will be true. func (tr *TableRestore) postProcess( ctx context.Context, - rc *RestoreController, + rc *Controller, cp *checkpoints.TableCheckpoint, forcePostProcess bool, ) (bool, error) { @@ -1705,7 +1705,7 @@ func (tr *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 @@ -1721,7 +1721,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 kv.ForAllStores( ctx, @@ -1733,16 +1733,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 @@ -1766,7 +1766,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) @@ -1851,7 +1851,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 @@ -1859,7 +1859,7 @@ func (rc *RestoreController) checkRequirements(ctx context.Context) error { return rc.backend.CheckRequirements(ctx) } -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 @@ -1868,13 +1868,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 { @@ -1897,7 +1897,7 @@ 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" } @@ -2003,7 +2003,7 @@ func (tr *TableRestore) Close() { tr.logger.Info("restore done") } -func (tr *TableRestore) populateChunks(ctx context.Context, rc *RestoreController, cp *checkpoints.TableCheckpoint) error { +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 { @@ -2150,7 +2150,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 { @@ -2164,7 +2164,7 @@ func getColumnNames(tableInfo *model.TableInfo, permutation []int) []string { func (tr *TableRestore) importKV( ctx context.Context, closedEngine *kv.ClosedEngine, - rc *RestoreController, + rc *Controller, engineID int32, ) error { task := closedEngine.Logger().Begin(zap.InfoLevel, "import and cleanup engine") @@ -2239,7 +2239,7 @@ func (cr *chunkRestore) deliverLoop( t *TableRestore, engineID int32, dataEngine, indexEngine *kv.LocalEngineWriter, - rc *RestoreController, + rc *Controller, ) (deliverTotalDur time.Duration, err error) { var channelClosed bool @@ -2329,7 +2329,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. @@ -2344,7 +2344,7 @@ func (cr *chunkRestore) deliverLoop( return } -func saveCheckpoint(rc *RestoreController, t *TableRestore, engineID int32, chunk *checkpoints.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. @@ -2381,7 +2381,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 { @@ -2479,7 +2479,7 @@ func (cr *chunkRestore) restore( t *TableRestore, engineID int32, dataEngine, indexEngine *kv.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 3f7eefdbd..7e85bf285 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -296,7 +296,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { 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]*checkpoints.EngineCheckpoint{ @@ -460,7 +460,7 @@ 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, &checkpoints.TableCheckpoint{}) c.Assert(err, IsNil) @@ -724,7 +724,7 @@ func (s *tableRestoreSuite) TestImportKVSuccess(c *C) { importer := kv.MakeBackend(mockBackend) chptCh := make(chan saveCp) defer close(chptCh) - rc := &RestoreController{saveCpCh: chptCh} + rc := &Controller{saveCpCh: chptCh} go func() { for range chptCh { } @@ -756,7 +756,7 @@ func (s *tableRestoreSuite) TestImportKVFailure(c *C) { importer := kv.MakeBackend(mockBackend) chptCh := make(chan saveCp) defer close(chptCh) - rc := &RestoreController{saveCpCh: chptCh} + rc := &Controller{saveCpCh: chptCh} go func() { for range chptCh { } @@ -812,7 +812,7 @@ func (s *chunkRestoreSuite) TearDownTest(c *C) { } func (s *chunkRestoreSuite) TestDeliverLoopCancel(c *C) { - rc := &RestoreController{backend: kv.NewMockImporter(nil, "")} + rc := &Controller{backend: kv.NewMockImporter(nil, "")} ctx, cancel := context.WithCancel(context.Background()) kvsCh := make(chan []deliveredKVs) @@ -851,7 +851,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{} @@ -944,7 +944,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) @@ -964,7 +964,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) @@ -991,7 +991,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) @@ -1011,7 +1011,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) @@ -1033,7 +1033,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) @@ -1050,7 +1050,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) @@ -1122,7 +1122,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, @@ -1136,7 +1136,7 @@ var _ = Suite(&restoreSchemaSuite{}) type restoreSchemaSuite struct { ctx context.Context - rc *RestoreController + rc *Controller controller *gomock.Controller } @@ -1178,7 +1178,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(), From bda1d1f83898ee686f2649e1ef553ad53d9b2d78 Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 18:35:29 +0800 Subject: [PATCH 06/29] lightning: disable noctx for tests --- pkg/lightning/lightning_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/lightning/lightning_test.go b/pkg/lightning/lightning_test.go index 4da5e73ad..c9f7555e5 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 ( From 38367154c46854a92351540445ff34adf3a4c6b3 Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 18:39:16 +0800 Subject: [PATCH 07/29] lightning: fixs rowsercheck --- pkg/lightning/checkpoints/checkpoints.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index c9ec53961..146f49572 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -524,6 +524,9 @@ func IsCheckpointsDBExists(ctx context.Context, cfg *config.Config) (bool, error if err != nil { return false, errors.Trace(err) } + if err := rows.Err(); err != nil { + return false, err + } defer rows.Close() return rows.Next(), nil @@ -1418,6 +1421,9 @@ func (cpdb *MySQLCheckpointsDB) DumpTables(ctx context.Context, writer io.Writer if err != nil { return errors.Trace(err) } + if err := rows.Err(); err != nil { + return err + } defer rows.Close() return errors.Trace(sqltocsv.Write(writer, rows)) @@ -1436,6 +1442,9 @@ func (cpdb *MySQLCheckpointsDB) DumpEngines(ctx context.Context, writer io.Write if err != nil { return errors.Trace(err) } + if err := rows.Err(); err != nil { + return err + } defer rows.Close() return errors.Trace(sqltocsv.Write(writer, rows)) @@ -1466,6 +1475,9 @@ func (cpdb *MySQLCheckpointsDB) DumpChunks(ctx context.Context, writer io.Writer if err != nil { return errors.Trace(err) } + if err := rows.Err(); err != nil { + return err + } defer rows.Close() return errors.Trace(sqltocsv.Write(writer, rows)) From 0b4938f0d072259817d116958e5c616e3f8cbe62 Mon Sep 17 00:00:00 2001 From: hillium Date: Mon, 1 Mar 2021 19:12:17 +0800 Subject: [PATCH 08/29] lightning: fixed test and fmt --- pkg/lightning/backend/local_test.go | 5 +++-- pkg/lightning/backend/sql2kv_test.go | 5 +++-- pkg/lightning/backend/tidb_test.go | 3 ++- pkg/lightning/mydump/loader_test.go | 2 +- pkg/lightning/mydump/reader_test.go | 2 +- pkg/lightning/mydump/region.go | 3 ++- pkg/lightning/web/progress.go | 5 ++--- 7 files changed, 14 insertions(+), 11 deletions(-) diff --git a/pkg/lightning/backend/local_test.go b/pkg/lightning/backend/local_test.go index 833c34d22..cec7321c0 100644 --- a/pkg/lightning/backend/local_test.go +++ b/pkg/lightning/backend/local_test.go @@ -465,11 +465,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/sql2kv_test.go b/pkg/lightning/backend/sql2kv_test.go index b41299033..635865b55 100644 --- a/pkg/lightning/backend/sql2kv_test.go +++ b/pkg/lightning/backend/sql2kv_test.go @@ -95,7 +95,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{ @@ -118,7 +118,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 @@ -399,6 +399,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/tidb_test.go b/pkg/lightning/backend/tidb_test.go index acb87b9ad..345cc965b 100644 --- a/pkg/lightning/backend/tidb_test.go +++ b/pkg/lightning/backend/tidb_test.go @@ -148,7 +148,8 @@ 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{ + // TODO: check row here? + _, 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}) diff --git a/pkg/lightning/mydump/loader_test.go b/pkg/lightning/mydump/loader_test.go index c29a6759a..21c7e534f 100644 --- a/pkg/lightning/mydump/loader_test.go +++ b/pkg/lightning/mydump/loader_test.go @@ -107,7 +107,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) { diff --git a/pkg/lightning/mydump/reader_test.go b/pkg/lightning/mydump/reader_test.go index 03bc37024..5c80dadfa 100644 --- a/pkg/lightning/mydump/reader_test.go +++ b/pkg/lightning/mydump/reader_test.go @@ -191,5 +191,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 7fc06d323..bca973781 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -298,7 +298,8 @@ func makeParquetFileRegion( if err != nil { return prevRowIdxMax, nil, errors.Trace(err) } - numberRows, err := ReadParquetFileRowCount(ctx, store, r, dataFile.FileMeta.Path) + // TODO: handle the error here? + numberRows, _ := ReadParquetFileRowCount(ctx, store, r, dataFile.FileMeta.Path) rowIDMax := prevRowIdxMax + numberRows region := &TableRegion{ DB: meta.DB, diff --git a/pkg/lightning/web/progress.go b/pkg/lightning/web/progress.go index 099594dda..5440692d7 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 = 2 ) type tableInfo struct { From 592559a48663d239b8eb1b2dc629ea9d5426d4eb Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 10:31:39 +0800 Subject: [PATCH 09/29] *: disable errorlint --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index 9e3187a69..b5517d3c1 100644 --- a/Makefile +++ b/Makefile @@ -188,6 +188,7 @@ 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. CGO_ENABLED=0 tools/bin/golangci-lint run --enable-all --deadline 120s \ --disable gochecknoglobals \ --disable goimports \ @@ -208,6 +209,7 @@ static: prepare tools --disable exhaustive \ --disable godot \ --disable gosec \ + --disable errorlint \ $$($(PACKAGE_DIRECTORIES)) # pingcap/errors APIs are mixed with multiple patterns 'pkg/errors', # 'juju/errors' and 'pingcap/parser'. To avoid confusion and mistake, From 929bb6485d1531b17f8b122642783c580184ca16 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 11:08:47 +0800 Subject: [PATCH 10/29] *: fix errorcheck --- cmd/tidb-lightning-ctl/main.go | 6 ++-- pkg/lightning/backend/session.go | 13 ++++++-- pkg/lightning/backend/sql2kv.go | 18 ++++++++--- pkg/lightning/backend/tidb.go | 4 ++- pkg/lightning/backend/tikv_test.go | 2 +- pkg/lightning/checkpoints/glue_checkpoint.go | 24 ++++++++++----- pkg/lightning/common/pause_test.go | 6 ++-- pkg/lightning/common/security_test.go | 6 ++-- pkg/lightning/lightning.go | 32 +++++++++++--------- pkg/lightning/lightning_test.go | 24 ++++++++++----- pkg/lightning/log/log.go | 4 ++- pkg/lightning/restore/restore.go | 14 ++++++--- pkg/lightning/restore/restore_test.go | 12 +++++--- 13 files changed, 108 insertions(+), 57 deletions(-) diff --git a/cmd/tidb-lightning-ctl/main.go b/cmd/tidb-lightning-ctl/main.go index 8854f60b5..f41a6b83e 100644 --- a/cmd/tidb-lightning-ctl/main.go +++ b/cmd/tidb-lightning-ctl/main.go @@ -58,7 +58,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") @@ -244,8 +244,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 } } } diff --git a/pkg/lightning/backend/session.go b/pkg/lightning/backend/session.go index 8088440ed..d72330638 100644 --- a/pkg/lightning/backend/session.go +++ b/pkg/lightning/backend/session.go @@ -17,6 +17,8 @@ import ( "context" "errors" "fmt" + "github.com/pingcap/br/pkg/lightning/log" + "go.uber.org/zap" "strconv" "github.com/pingcap/parser/model" @@ -190,11 +192,18 @@ func newSession(options *SessionOptions) *session { vars.StmtCtx.IgnoreZeroInDate = !sqlMode.HasStrictMode() || sqlMode.HasAllowInvalidDatesMode() if options.SysVars != nil { for k, v := range options.SysVars { - vars.SetSystemVar(k, v) + if err := vars.SetSystemVar(k, v); err != nil { + log.L().Warn("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/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 7cc1ad54c..bd8b0fe7f 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -212,11 +212,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 } @@ -334,10 +336,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 { diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 6b0bcc860..be8416d59 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -210,7 +210,9 @@ func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, _ *ta 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 err + } sb.WriteByte('\'') case types.KindMysqlBit: diff --git a/pkg/lightning/backend/tikv_test.go b/pkg/lightning/backend/tikv_test.go index a74b42c79..b6ebe7395 100644 --- a/pkg/lightning/backend/tikv_test.go +++ b/pkg/lightning/backend/tikv_test.go @@ -20,7 +20,7 @@ var _ = Suite(&tikvSuite{}) func (s *tikvSuite) TestForAllStores(c *C) { server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { - w.Write([]byte(` + _, _ = w.Write([]byte(` { "count": 5, "stores": [ diff --git a/pkg/lightning/checkpoints/glue_checkpoint.go b/pkg/lightning/checkpoints/glue_checkpoint.go index da987d2e0..337c271c9 100644 --- a/pkg/lightning/checkpoints/glue_checkpoint.go +++ b/pkg/lightning/checkpoints/glue_checkpoint.go @@ -56,6 +56,14 @@ type GlueCheckpointsDB struct { 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 common.WriteMySQLIdentifier(&escapedSchemaName, schemaName) @@ -126,7 +134,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), @@ -146,7 +154,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 { @@ -354,13 +362,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{ @@ -420,22 +428,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 { diff --git a/pkg/lightning/common/pause_test.go b/pkg/lightning/common/pause_test.go index e235693fd..7f45017bf 100644 --- a/pkg/lightning/common/pause_test.go +++ b/pkg/lightning/common/pause_test.go @@ -138,7 +138,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) } } @@ -148,7 +148,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) } } @@ -176,6 +176,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/lightning.go b/pkg/lightning/lightning.go index 64d639c4c..69f088eaf 100755 --- a/pkg/lightning/lightning.go +++ b/pkg/lightning/lightning.go @@ -257,7 +257,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 @@ -344,7 +346,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) { @@ -414,7 +416,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) { @@ -481,7 +483,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) { @@ -514,7 +516,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) } @@ -545,7 +547,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) } @@ -553,15 +555,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) { @@ -571,7 +573,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()) } } @@ -587,7 +589,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()) } } @@ -603,7 +605,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) @@ -619,7 +621,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) @@ -638,7 +640,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 { @@ -649,7 +651,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) diff --git a/pkg/lightning/lightning_test.go b/pkg/lightning/lightning_test.go index c9f7555e5..83efa80c0 100644 --- a/pkg/lightning/lightning_test.go +++ b/pkg/lightning/lightning_test.go @@ -118,13 +118,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() } @@ -141,7 +141,10 @@ func (s *lightningServerSuite) TestRunServer(c *C) { c.Assert(data["error"], Equals, "server-mode not enabled") resp.Body.Close() - go s.lightning.RunServer() + go func() { + err := s.lightning.RunServer() + c.Assert(err, IsNil) + }() time.Sleep(100 * time.Millisecond) req, err := http.NewRequest(http.MethodPut, url, nil) @@ -228,7 +231,10 @@ func (s *lightningServerSuite) TestGetDeleteTask(c *C) { return result.ID } - go s.lightning.RunServer() + go func() { + err := s.lightning.RunServer() + c.Assert(err, IsNil) + }() time.Sleep(100 * time.Millisecond) // Check `GET /tasks` without any active tasks @@ -474,7 +480,9 @@ func (s *lightningServerSuite) TestCheckSystemRequirement(c *C) { c.Assert(err, IsNil) err = failpoint.Enable("github.com/pingcap/br/pkg/lightning/backend/SetRlimitError", "return(true)") c.Assert(err, IsNil) - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/SetRlimitError") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/SetRlimitError") + }() // with this dbMetas, the estimated fds will be 2050, so should return error err = checkSystemRequirement(cfg, dbMetas) c.Assert(err, NotNil) @@ -490,7 +498,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/GetRlimitValue", "return(8200)") - defer failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/GetRlimitValue") + defer func() { + _ = failpoint.Disable("github.com/pingcap/br/pkg/lightning/backend/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 e3b8f3328..1dd5f2dd0 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 err + } logCfg := &pclog.Config{ Level: cfg.Level, diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index c7dc40649..c3c87eac6 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -829,7 +829,9 @@ func (rc *Controller) listenCheckpointUpdates() { failpoint.Inject("KillIfImportedChunk", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { - common.KillMySelf() + if err := common.KillMySelf(); err != nil { + log.L().Warn("KillMySelf() failed to kill itself", log.ShortError(err)) + } } }) } @@ -1221,10 +1223,14 @@ func (tr *TableRestore) restoreTable( // rebase the allocator so it exceeds the number of rows. if tr.tableInfo.Core.PKIsHandle && tr.tableInfo.Core.ContainsAutoRandomBits() { cp.AllocBase = mathutil.MaxInt64(cp.AllocBase, tr.tableInfo.Core.AutoRandID) - tr.alloc.Get(autoid.AutoRandomType).Rebase(tr.tableInfo.ID, cp.AllocBase, false) + 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, tr.tableInfo.Core.AutoIncID) - tr.alloc.Get(autoid.RowIDAllocType).Rebase(tr.tableInfo.ID, cp.AllocBase, false) + if err := tr.alloc.Get(autoid.RowIDAllocType).Rebase(tr.tableInfo.ID, cp.AllocBase, false); err != nil { + return false, err + } } rc.saveCpCh <- saveCp{ tableName: tr.tableName, @@ -2172,7 +2178,7 @@ func (tr *TableRestore) importKV( err := closedEngine.Import(ctx) rc.saveStatusCheckpoint(tr.tableName, engineID, err, checkpoints.CheckpointStatusImported) if err == nil { - closedEngine.Cleanup(ctx) + err = closedEngine.Cleanup(ctx) } dur := task.End(zap.ErrorLevel, err) diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index 7e85bf285..292601304 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -289,8 +289,10 @@ 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 := &checkpoints.TableCheckpoint{ Engines: make(map[int32]*checkpoints.EngineCheckpoint), @@ -447,8 +449,10 @@ 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 := &checkpoints.TableCheckpoint{ Engines: make(map[int32]*checkpoints.EngineCheckpoint), From b552dfbe96f39d044e538bb7687af13cf042e7ec Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 11:28:30 +0800 Subject: [PATCH 11/29] *: fix deadcode --- pkg/lightning/backend/session.go | 6 ++++-- pkg/lightning/config/config.go | 1 - pkg/lightning/mydump/region_test.go | 15 --------------- pkg/lightning/restore/const.go | 18 ------------------ 4 files changed, 4 insertions(+), 36 deletions(-) delete mode 100644 pkg/lightning/restore/const.go diff --git a/pkg/lightning/backend/session.go b/pkg/lightning/backend/session.go index d72330638..4b0ac4331 100644 --- a/pkg/lightning/backend/session.go +++ b/pkg/lightning/backend/session.go @@ -17,10 +17,12 @@ import ( "context" "errors" "fmt" - "github.com/pingcap/br/pkg/lightning/log" - "go.uber.org/zap" "strconv" + "go.uber.org/zap" + + "github.com/pingcap/br/pkg/lightning/log" + "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/kv" diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index 1b2c39ce9..0b9291041 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -95,7 +95,6 @@ const ( ) var ( - defaultConfigPaths = []string{"tidb-lightning.toml", "conf/tidb-lightning.toml"} supportedStorageTypes = []string{"file", "local", "s3", "noop"} DefaultFilter = []string{ 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/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 -) From daa8d54f7dad43d34a667a7cceb4c9656f17e25e Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 11:37:26 +0800 Subject: [PATCH 12/29] lightning: fix structcheck --- pkg/lightning/backend/local.go | 8 +++----- pkg/lightning/checkpoints/checkpoints_file_test.go | 2 -- pkg/lightning/restore/restore.go | 1 - pkg/lightning/restore/tidb_test.go | 8 +++----- 4 files changed, 6 insertions(+), 13 deletions(-) diff --git a/pkg/lightning/backend/local.go b/pkg/lightning/backend/local.go index 0d07e8d9f..874c7c286 100644 --- a/pkg/lightning/backend/local.go +++ b/pkg/lightning/backend/local.go @@ -99,9 +99,8 @@ 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. @@ -348,7 +347,6 @@ type connPool struct { mu sync.Mutex conns []*grpc.ClientConn - name string next int cap int newConn func(ctx context.Context) (*grpc.ClientConn, error) @@ -927,7 +925,7 @@ func (local *local) readAndSplitIntoRange(engineFile *LocalFile) ([]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 } diff --git a/pkg/lightning/checkpoints/checkpoints_file_test.go b/pkg/lightning/checkpoints/checkpoints_file_test.go index e49a9d738..25ced0040 100644 --- a/pkg/lightning/checkpoints/checkpoints_file_test.go +++ b/pkg/lightning/checkpoints/checkpoints_file_test.go @@ -21,9 +21,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/restore/restore.go b/pkg/lightning/restore/restore.go index c3c87eac6..25275bd18 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -158,7 +158,6 @@ type Controller struct { closedEngineLimit *worker.Pool store storage.ExternalStorage - checksumManager ChecksumManager diskQuotaLock sync.RWMutex diskQuotaState int32 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) { From b123583e552f9f6f145492638720399531d17c1b Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 12:07:23 +0800 Subject: [PATCH 13/29] lightning: fix duplicated code and cyclomatic complexity too high --- pkg/lightning/config/config.go | 286 +++++++++++++++----------- pkg/lightning/mydump/reader_test.go | 42 ++-- pkg/lightning/restore/restore_test.go | 1 + 3 files changed, 176 insertions(+), 153 deletions(-) diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index 0b9291041..a40178fca 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -538,79 +538,17 @@ 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 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) } 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*autoDiskQuotaLocalReservedSize + 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 } } @@ -629,25 +567,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. @@ -667,6 +588,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(err) +} + +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*autoDiskQuotaLocalReservedSize + 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) + } + 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() @@ -694,47 +701,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(err error) error { var u *url.URL // An absolute Windows path like "C:\Users\XYZ" would be interpreted as @@ -778,7 +748,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/mydump/reader_test.go b/pkg/lightning/mydump/reader_test.go index 5c80dadfa..c66a3686e 100644 --- a/pkg/lightning/mydump/reader_test.go +++ b/pkg/lightning/mydump/reader_test.go @@ -57,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 @@ -71,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 @@ -101,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) @@ -113,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) { diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index 292601304..780596599 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -301,6 +301,7 @@ func (s *tableRestoreSuite) TestPopulateChunks(c *C) { 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) + //nolint:dupl // false positive. c.Assert(cp.Engines, DeepEquals, map[int32]*checkpoints.EngineCheckpoint{ -1: { Status: checkpoints.CheckpointStatusLoaded, From 5200410919b330647c242322a54a18bdb0e96871 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 12:52:00 +0800 Subject: [PATCH 14/29] lightning: fix noinit and govet --- pkg/lightning/backend/local.go | 8 ++---- pkg/lightning/backend/sql2kv.go | 6 ++--- pkg/lightning/backend/tidb.go | 6 +++-- pkg/lightning/backend/tidb_test.go | 1 + pkg/lightning/checkpoints/checkpoints.go | 17 +++++-------- pkg/lightning/common/version_test.go | 12 +++++---- pkg/lightning/metric/metric.go | 1 + pkg/lightning/mydump/csv_parser.go | 1 + pkg/lightning/restore/restore.go | 1 + pkg/lightning/restore/restore_test.go | 32 ++++++++++++++++++------ 10 files changed, 50 insertions(+), 35 deletions(-) diff --git a/pkg/lightning/backend/local.go b/pkg/lightning/backend/local.go index 874c7c286..622da5c5e 100644 --- a/pkg/lightning/backend/local.go +++ b/pkg/lightning/backend/local.go @@ -954,12 +954,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 { diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index bd8b0fe7f..792ef4db2 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -429,16 +429,16 @@ func (kvs kvPairs) SplitIntoChunks(splitSize int) []Rows { for j, pair := range kvs { size := len(pair.Key) + len(pair.Val) if i < j && cumSize+size > splitSize { - res = append(res, kvPairs(kvs[i:j])) + res = append(res, kvs[i:j]) i = j cumSize = 0 } cumSize += size } - return append(res, kvPairs(kvs[i:])) + return append(res, kvs[i:]) } func (kvs kvPairs) Clear() Rows { - return kvPairs(kvs[:0]) + return kvs[:0] } diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index be8416d59..99bfec5ed 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -87,7 +87,7 @@ func NewTiDBBackend(db *sql.DB, onDuplicate string) Backend { func (row tidbRow) ClassifyAndAppend(data *Rows, checksum *verification.KVChecksum, _ *Rows, _ *verification.KVChecksum) { rows := (*data).(tidbRows) - *data = tidbRows(append(rows, row)) + *data = append(rows, row) cs := verification.MakeKVChecksum(uint64(len(row)), 1, 0) checksum.Add(&cs) } @@ -540,7 +540,9 @@ func (be *tidbBackend) FetchRemoteTableModels(ctx context.Context, schemaName st } } } - rows.Close() + if err := rows.Close(); err != nil { + return err + } if rows.Err() != nil { return rows.Err() } diff --git a/pkg/lightning/backend/tidb_test.go b/pkg/lightning/backend/tidb_test.go index 345cc965b..93f77554d 100644 --- a/pkg/lightning/backend/tidb_test.go +++ b/pkg/lightning/backend/tidb_test.go @@ -187,6 +187,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 146f49572..4ec2039fd 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -524,11 +524,12 @@ func IsCheckpointsDBExists(ctx context.Context, cfg *config.Config) (bool, error if err != nil { return false, errors.Trace(err) } + defer rows.Close() + result := rows.Next() if err := rows.Err(); err != nil { return false, err } - defer rows.Close() - return rows.Next(), nil + return result, nil case config.CheckpointDriverFile: _, err := os.Stat(cfg.Checkpoint.DSN) @@ -1406,6 +1407,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 @@ -1421,14 +1423,12 @@ func (cpdb *MySQLCheckpointsDB) DumpTables(ctx context.Context, writer io.Writer if err != nil { return errors.Trace(err) } - if err := rows.Err(); err != nil { - return err - } defer rows.Close() 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 @@ -1442,14 +1442,12 @@ func (cpdb *MySQLCheckpointsDB) DumpEngines(ctx context.Context, writer io.Write if err != nil { return errors.Trace(err) } - if err := rows.Err(); err != nil { - return err - } defer rows.Close() 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 @@ -1475,9 +1473,6 @@ func (cpdb *MySQLCheckpointsDB) DumpChunks(ctx context.Context, writer io.Writer if err != nil { return errors.Trace(err) } - if err := rows.Err(); err != nil { - return err - } defer rows.Close() return errors.Trace(sqltocsv.Write(writer, rows)) diff --git a/pkg/lightning/common/version_test.go b/pkg/lightning/common/version_test.go index 8b629fc39..d024d17ab 100644 --- a/pkg/lightning/common/version_test.go +++ b/pkg/lightning/common/version_test.go @@ -20,6 +20,8 @@ import ( "github.com/pingcap/br/pkg/lightning/common" ) +const None = "None" + func (s *utilSuite) TestVersion(c *C) { common.ReleaseVersion = "ReleaseVersion" common.BuildTS = "BuildTS" @@ -35,11 +37,11 @@ UTC Build Time: BuildTS Go Version: GoVersion `) common.PrintInfo("test", func() { - common.ReleaseVersion = "None" - common.BuildTS = "None" - common.GitHash = "None" - common.GitBranch = "None" - common.GoVersion = "None" + common.ReleaseVersion = None + common.BuildTS = None + common.GitHash = None + common.GitBranch = None + common.GoVersion = None }) version = common.GetRawInfo() 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 b7f2e6e7a..7ff08f09f 100644 --- a/pkg/lightning/mydump/csv_parser.go +++ b/pkg/lightning/mydump/csv_parser.go @@ -130,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) { diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 25275bd18..24bf80586 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -73,6 +73,7 @@ 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) { diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index 780596599..cdcfea474 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -259,21 +259,37 @@ 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, } } @@ -437,7 +453,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) From 99c75de0519bd44d71b4bc82916a744cfea82bbc Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 13:22:06 +0800 Subject: [PATCH 15/29] lightning: fixed goconst, nakedret, interfacer --- cmd/tidb-lightning-ctl/main.go | 1 + pkg/lightning/backend/backend_test.go | 3 ++- pkg/lightning/backend/importer.go | 1 + pkg/lightning/backend/sql2kv.go | 1 + pkg/lightning/backend/tidb.go | 4 +++ pkg/lightning/checkpoints/checkpoints.go | 27 ++++++++++++-------- pkg/lightning/checkpoints/glue_checkpoint.go | 16 ++++++------ pkg/lightning/config/config.go | 1 + pkg/lightning/restore/restore.go | 2 ++ pkg/lightning/restore/restore_test.go | 13 +++++++--- pkg/lightning/web/progress.go | 2 +- 11 files changed, 47 insertions(+), 24 deletions(-) diff --git a/cmd/tidb-lightning-ctl/main.go b/cmd/tidb-lightning-ctl/main.go index f41a6b83e..f09aac54e 100644 --- a/cmd/tidb-lightning-ctl/main.go +++ b/cmd/tidb-lightning-ctl/main.go @@ -318,6 +318,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/pkg/lightning/backend/backend_test.go b/pkg/lightning/backend/backend_test.go index 64297af24..5c9a706a0 100644 --- a/pkg/lightning/backend/backend_test.go +++ b/pkg/lightning/backend/backend_test.go @@ -25,7 +25,7 @@ var _ = Suite(&backendSuite{}) // 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 = kv.MakeBackend(s.mockBackend) @@ -273,6 +273,7 @@ func (s *backendSuite) TestImportFailedRecovered(c *C) { c.Assert(err, IsNil) } +//nolint:interfacer // change test case signature might cause Check failed to find this function? func (s *backendSuite) TestClose(c *C) { s.setUpTest(c) defer s.tearDownTest() diff --git a/pkg/lightning/backend/importer.go b/pkg/lightning/backend/importer.go index de1b686f5..db585838d 100644 --- a/pkg/lightning/backend/importer.go +++ b/pkg/lightning/backend/importer.go @@ -194,6 +194,7 @@ outside: func (importer *importer) WriteRowsToImporter( ctx context.Context, + //nolint:interfacer // false positive engineUUID uuid.UUID, ts uint64, rows Rows, diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 792ef4db2..951ee5b2f 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -291,6 +291,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 { diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 99bfec5ed..3368f5170 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -87,6 +87,9 @@ func NewTiDBBackend(db *sql.DB, onDuplicate string) Backend { func (row tidbRow) ClassifyAndAppend(data *Rows, checksum *verification.KVChecksum, _ *Rows, _ *verification.KVChecksum) { rows := (*data).(tidbRows) + // I'm not sure if `rows := data.(*tidbRows); *rows = append(*rows, row)` could solve this lint. + // Seems there are some stories. Leave it untouched. + //nolint:gocritic *data = append(rows, row) cs := verification.MakeKVChecksum(uint64(len(row)), 1, 0) checksum.Add(&cs) @@ -418,6 +421,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, diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index 4ec2039fd..d6c58264e 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -64,6 +64,11 @@ const ( CheckpointTableNameTable = "table_v6" CheckpointTableNameEngine = "engine_v5" CheckpointTableNameChunk = "chunk_v5" + + // Some frequently used table name or constants. + TableNameAll = "all" + StringLitAll = "'all'" + ColTableName = "table_name" ) const ( @@ -1200,7 +1205,7 @@ func (cpdb *MySQLCheckpointsDB) RemoveCheckpoint(ctx context.Context, tableName Logger: log.With(zap.String("table", tableName)), } - if tableName == "all" { + if tableName == TableNameAll { return s.Exec(ctx, "remove all checkpoints", "DROP SCHEMA "+cpdb.schema) } @@ -1299,12 +1304,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 == TableNameAll { // This will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" + colName = StringLitAll } else { - colName = "table_name" + colName = ColTableName } engineQuery := fmt.Sprintf(` @@ -1333,13 +1338,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 == TableNameAll { // 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 = ColTableName aliasedColName = "t.table_name" } @@ -1482,7 +1487,7 @@ func (cpdb *FileCheckpointsDB) RemoveCheckpoint(_ context.Context, tableName str cpdb.lock.Lock() defer cpdb.lock.Unlock() - if tableName == "all" { + if tableName == TableNameAll { cpdb.checkpoints.Reset() return errors.Trace(os.Remove(cpdb.path)) } @@ -1533,7 +1538,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 == TableNameAll || targetTableName == tableName) { continue } if tableModel.Status <= uint32(CheckpointStatusMaxInvalid) { @@ -1556,7 +1561,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 == TableNameAll || targetTableName == tableName) { continue } if tableModel.Status <= uint32(CheckpointStatusMaxInvalid) { diff --git a/pkg/lightning/checkpoints/glue_checkpoint.go b/pkg/lightning/checkpoints/glue_checkpoint.go index 337c271c9..67ed770db 100644 --- a/pkg/lightning/checkpoints/glue_checkpoint.go +++ b/pkg/lightning/checkpoints/glue_checkpoint.go @@ -513,7 +513,7 @@ func (g GlueCheckpointsDB) RemoveCheckpoint(ctx context.Context, tableName strin } defer se.Close() - if tableName == "all" { + if tableName == TableNameAll { return common.Retry("remove all checkpoints", logger, func() error { _, err := se.Execute(ctx, "DROP SCHEMA "+g.schema) return err @@ -632,12 +632,12 @@ func (g GlueCheckpointsDB) IgnoreErrorCheckpoint(ctx context.Context, tableName defer se.Close() var colName string - if tableName == "all" { + if tableName == TableNameAll { // This will expand to `WHERE 'all' = 'all'` and effectively allowing // all tables to be included. - colName = "'all'" + colName = StringLitAll } else { - colName = "table_name" + colName = ColTableName } tableName = common.InterpolateMySQLString(tableName) @@ -669,13 +669,13 @@ func (g GlueCheckpointsDB) DestroyErrorCheckpoint(ctx context.Context, tableName var colName, aliasedColName string - if tableName == "all" { + if tableName == TableNameAll { // 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 = ColTableName aliasedColName = "t.table_name" } diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index a40178fca..86360388f 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -197,6 +197,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": diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 24bf80586..e9fabafd7 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -2239,6 +2239,7 @@ type deliverResult struct { err error } +//nolint:nakedret // TODO: refactor func (cr *chunkRestore) deliverLoop( ctx context.Context, kvsCh <-chan []deliveredKVs, @@ -2380,6 +2381,7 @@ func saveCheckpoint(rc *Controller, t *TableRestore, engineID int32, chunk *chec } } +//nolint:nakedret // TODO: refactor func (cr *chunkRestore) encodeLoop( ctx context.Context, kvsCh chan<- []deliveredKVs, diff --git a/pkg/lightning/restore/restore_test.go b/pkg/lightning/restore/restore_test.go index cdcfea474..6a297af45 100644 --- a/pkg/lightning/restore/restore_test.go +++ b/pkg/lightning/restore/restore_test.go @@ -265,7 +265,9 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { Path: fakeFileName, Type: mydump.SourceTypeSQL, SortKey: fmt.Sprintf("%d", i), - FileSize: 37}}) + FileSize: 37, + }, + }) } fakeCsvContent := []byte("1,2,3\r\n4,5,6\r\n") @@ -278,7 +280,9 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { Path: csvName, Type: mydump.SourceTypeCSV, SortKey: "99", - FileSize: 14}}) + FileSize: 14, + }, + }) s.tableMeta = &mydump.MDTableMeta{ DB: "db", @@ -288,7 +292,9 @@ func (s *tableRestoreSuiteBase) SetUpSuite(c *C) { TableName: filter.Table{Schema: "db", Name: "table"}, FileMeta: mydump.SourceFileMeta{ Path: "db.table-schema.sql", - Type: mydump.SourceTypeTableSchema}}, + Type: mydump.SourceTypeTableSchema, + }, + }, DataFiles: fakeDataFiles, } } @@ -1207,6 +1213,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/web/progress.go b/pkg/lightning/web/progress.go index 5440692d7..d109f7ed3 100644 --- a/pkg/lightning/web/progress.go +++ b/pkg/lightning/web/progress.go @@ -89,7 +89,7 @@ type taskStatus uint8 const ( taskStatusRunning taskStatus = 1 - taskStatusCompleted = 2 + taskStatusCompleted taskStatus = 2 ) type tableInfo struct { From 38600e61b9e5ca2f4532888158e32d7623a91dfe Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 14:03:22 +0800 Subject: [PATCH 16/29] lightning: fixed scopelint and static check --- pkg/lightning/backend/localhelper.go | 27 ++++++++++++------------ pkg/lightning/backend/tidb.go | 7 ++++-- pkg/lightning/checkpoints/checkpoints.go | 5 +++-- pkg/lightning/config/config.go | 11 +++++----- pkg/lightning/lightning_test.go | 2 +- pkg/lightning/mydump/loader.go | 11 +++++----- pkg/lightning/mydump/parser.go | 9 +++++--- pkg/lightning/restore/restore.go | 7 ++++-- 8 files changed, 46 insertions(+), 33 deletions(-) diff --git a/pkg/lightning/backend/localhelper.go b/pkg/lightning/backend/localhelper.go index 1051c620d..79f688839 100644 --- a/pkg/lightning/backend/localhelper.go +++ b/pkg/lightning/backend/localhelper.go @@ -126,25 +126,26 @@ func (local *local) SplitAndScatterRegionByRanges(ctx context.Context, ranges [] for regionID, keys := range splitKeyMap { var newRegions []*split.RegionInfo region := regionMap[regionID] - sort.Slice(keys, func(i, j int) bool { - return bytes.Compare(keys[i], keys[j]) < 0 + ks := keys + sort.Slice(ks, func(i, j int) bool { + return bytes.Compare(ks[i], ks[j]) < 0 }) splitRegion := region - for j := 0; j < (len(keys)+maxBatchSplitKeys-1)/maxBatchSplitKeys; j++ { + for j := 0; j < (len(ks)+maxBatchSplitKeys-1)/maxBatchSplitKeys; j++ { start := j * maxBatchSplitKeys - end := utils.MinInt((j+1)*maxBatchSplitKeys, len(keys)) - splitRegionStart := codec.EncodeBytes([]byte{}, keys[start]) - splitRegionEnd := codec.EncodeBytes([]byte{}, keys[end-1]) + end := utils.MinInt((j+1)*maxBatchSplitKeys, len(ks)) + splitRegionStart := codec.EncodeBytes([]byte{}, ks[start]) + splitRegionEnd := codec.EncodeBytes([]byte{}, ks[end-1]) if bytes.Compare(splitRegionStart, splitRegion.Region.StartKey) < 0 || !beforeEnd(splitRegionEnd, splitRegion.Region.EndKey) { log.L().Fatal("no valid key in region", log.ZapRedactBinary("startKey", splitRegionStart), log.ZapRedactBinary("endKey", splitRegionEnd), log.ZapRedactBinary("regionStart", splitRegion.Region.StartKey), log.ZapRedactBinary("regionEnd", splitRegion.Region.EndKey), log.ZapRedactReflect("region", splitRegion)) } - splitRegion, newRegions, err = local.BatchSplitRegions(ctx, splitRegion, keys[start:end]) + splitRegion, newRegions, err = local.BatchSplitRegions(ctx, splitRegion, ks[start:end]) if err != nil { if strings.Contains(err.Error(), "no valid key") { - for _, key := range keys { + for _, key := range ks { log.L().Warn("no valid key", log.ZapRedactBinary("startKey", region.Region.StartKey), log.ZapRedactBinary("endKey", region.Region.EndKey), @@ -154,12 +155,12 @@ func (local *local) SplitAndScatterRegionByRanges(ctx context.Context, ranges [] } log.L().Warn("split regions", log.ShortError(err), zap.Int("retry time", j+1), zap.Uint64("region_id", regionID)) - retryKeys = append(retryKeys, keys[start:]...) + retryKeys = append(retryKeys, ks[start:]...) break } else { log.L().Info("batch split region", zap.Uint64("region_id", splitRegion.Region.Id), - zap.Int("keys", end-start), zap.Binary("firstKey", keys[start]), - zap.Binary("end", keys[end-1])) + zap.Int("keys", end-start), zap.Binary("firstKey", ks[start]), + zap.Binary("end", ks[end-1])) sort.Slice(newRegions, func(i, j int) bool { return bytes.Compare(newRegions[i].Region.StartKey, newRegions[j].Region.StartKey) < 0 }) @@ -278,7 +279,7 @@ func (local *local) BatchSplitRegions(ctx context.Context, region *split.RegionI func (local *local) hasRegion(ctx context.Context, regionID uint64) (bool, error) { regionInfo, err := local.splitCli.GetRegionByID(ctx, regionID) if err != nil { - return false, err + return false, errors.Trace(err) } return regionInfo != nil, nil } @@ -324,7 +325,7 @@ func (local *local) waitForScatterRegion(ctx context.Context, regionInfo *split. func (local *local) isScatterRegionFinished(ctx context.Context, regionID uint64) (bool, error) { resp, err := local.splitCli.GetOperator(ctx, regionID) if err != nil { - return false, err + return false, errors.Trace(err) } // Heartbeat may not be sent to PD if respErr := resp.GetHeader().GetError(); respErr != nil { diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 3368f5170..4e33d58f9 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -214,7 +214,7 @@ func (enc *tidbEncoder) appendSQL(sb *strings.Builder, datum *types.Datum, _ *ta sb.Grow(3 + 2*len(value)) sb.WriteString("x'") if _, err := hex.NewEncoder(sb).Write(value); err != nil { - return err + return errors.Trace(err) } sb.WriteByte('\'') @@ -282,7 +282,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", rowArrayMarshaler(row)), zap.Int("originalCol", i), @@ -544,6 +545,8 @@ func (be *tidbBackend) FetchRemoteTableModels(ctx context.Context, schemaName st } } } + // 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 err } diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index d6c58264e..f07223369 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -364,14 +364,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 diff --git a/pkg/lightning/config/config.go b/pkg/lightning/config/config.go index 86360388f..84cf83254 100644 --- a/pkg/lightning/config/config.go +++ b/pkg/lightning/config/config.go @@ -319,7 +319,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) @@ -338,7 +338,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) { @@ -594,7 +594,7 @@ func (cfg *Config) Adjust(ctx context.Context) error { } cfg.AdjustMydumper() cfg.AdjustCheckPoint() - return cfg.CheckAndAdjustFilePath(err) + return cfg.CheckAndAdjustFilePath() } func (cfg *Config) CheckAndAdjustForLocalBackend() error { @@ -624,7 +624,7 @@ func (cfg *Config) CheckAndAdjustForLocalBackend() error { storageSize, err := common.GetStorageSize(storageSizeDir) if err != nil { - return err + return errors.Trace(err) } if storageSize.Available <= reservedSize { return errors.Errorf( @@ -705,7 +705,7 @@ func (cfg *Config) CheckAndAdjustTiDBPort(ctx context.Context, mustHaveInternalC return nil } -func (cfg *Config) CheckAndAdjustFilePath(err error) error { +func (cfg *Config) CheckAndAdjustFilePath() error { var u *url.URL // An absolute Windows path like "C:\Users\XYZ" would be interpreted as @@ -717,6 +717,7 @@ func (cfg *Config) CheckAndAdjustFilePath(err error) 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) diff --git a/pkg/lightning/lightning_test.go b/pkg/lightning/lightning_test.go index 83efa80c0..d44d72bec 100644 --- a/pkg/lightning/lightning_test.go +++ b/pkg/lightning/lightning_test.go @@ -423,7 +423,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) } diff --git a/pkg/lightning/mydump/loader.go b/pkg/lightning/mydump/loader.go index 6728b41e6..eebe895eb 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.Create(ctx, u, true) if err != nil { - return nil, err + return nil, errors.Trace(err) } return NewMyDumpLoaderWithStore(ctx, cfg, s) @@ -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 diff --git a/pkg/lightning/mydump/parser.go b/pkg/lightning/mydump/parser.go index e1d6ef097..e5275c12d 100644 --- a/pkg/lightning/mydump/parser.go +++ b/pkg/lightning/mydump/parser.go @@ -276,7 +276,7 @@ func unescape( ) string { if len(delim) > 0 { delim2 := delim + delim - if strings.Index(input, delim2) != -1 { + if strings.Contains(input, delim2) { input = strings.ReplaceAll(input, delim2, delim) } } @@ -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/restore/restore.go b/pkg/lightning/restore/restore.go index e9fabafd7..e0df2c43d 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -801,6 +801,7 @@ func (rc *Controller) listenCheckpointUpdates() { lock.Unlock() + //nolint:scopelint // This would be either INLINED or ERASED, believe me! failpoint.Inject("FailIfImportedChunk", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { rc.checkpointsWg.Done() @@ -809,6 +810,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) + //nolint:scopelint // This would be either INLINED or ERASED, believe me! failpoint.Inject("FailIfStatusBecomes", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && merger.EngineID >= 0 && int(merger.Status) == val.(int) { rc.checkpointsWg.Done() @@ -817,6 +819,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) + //nolint:scopelint // This would be either INLINED or ERASED, believe me! failpoint.Inject("FailIfIndexEngineImported", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && merger.EngineID == checkpoints.WholeTableEngineID && @@ -827,6 +830,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) + //nolint:scopelint // This would be either INLINED or ERASED, believe me! failpoint.Inject("KillIfImportedChunk", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { if err := common.KillMySelf(); err != nil { @@ -915,7 +919,6 @@ func (rc *Controller) runPeriodicActions(ctx context.Context, stop <-chan struct } else { state = "post-processing" } - remaining = zap.Skip() case finished > 0: state = "writing" default: @@ -1168,7 +1171,7 @@ func (rc *Controller) restoreTables(ctx context.Context) error { err = ctx.Err() } logTask.End(zap.ErrorLevel, err) - return err + return errors.Trace(err) default: } From bcc6528324cc7689be74d5bc5008f6cb484b2298 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 15:04:35 +0800 Subject: [PATCH 17/29] lightning: disabled wrapcheck --- Makefile | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile b/Makefile index b5517d3c1..569f44454 100644 --- a/Makefile +++ b/Makefile @@ -189,6 +189,7 @@ static: prepare tools @# 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 \ @@ -210,6 +211,7 @@ static: prepare tools --disable godot \ --disable gosec \ --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, From 35b11bbf22c6901ab03817e4d8f1a26cfd6d3aee Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 2 Mar 2021 15:25:07 +0800 Subject: [PATCH 18/29] *: fix make file format --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 569f44454..eefaa1820 100644 --- a/Makefile +++ b/Makefile @@ -189,7 +189,7 @@ static: prepare tools @# 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 + @# 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 \ From caf5e22827c5d40768ff83e6c8758a2d6e9000bb Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 9 Mar 2021 10:36:22 +0800 Subject: [PATCH 19/29] lightning: address comments --- pkg/lightning/backend/backend_test.go | 3 +-- pkg/lightning/backend/session.go | 2 +- pkg/lightning/backend/tidb.go | 3 +-- pkg/lightning/mydump/region.go | 6 ++++-- pkg/lightning/restore/restore.go | 8 ++++---- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/lightning/backend/backend_test.go b/pkg/lightning/backend/backend_test.go index 3e091ea71..725f5b225 100644 --- a/pkg/lightning/backend/backend_test.go +++ b/pkg/lightning/backend/backend_test.go @@ -273,8 +273,7 @@ func (s *backendSuite) TestImportFailedRecovered(c *C) { c.Assert(err, IsNil) } -//nolint:interfacer // change test case signature might cause Check failed to find this function? -func (s *backendSuite) TestClose(c *C) { +func (s *backendSuite) TestClose(c gomock.TestReporter) { s.setUpTest(c) defer s.tearDownTest() diff --git a/pkg/lightning/backend/session.go b/pkg/lightning/backend/session.go index 0222b4683..b50e57b21 100644 --- a/pkg/lightning/backend/session.go +++ b/pkg/lightning/backend/session.go @@ -199,7 +199,7 @@ func newSession(options *SessionOptions) *session { if options.SysVars != nil { for k, v := range options.SysVars { if err := vars.SetSystemVar(k, v); err != nil { - log.L().Warn("new session: failed to set system var", + log.L().DPanic("new session: failed to set system var", log.ShortError(err), zap.String("key", k)) } diff --git a/pkg/lightning/backend/tidb.go b/pkg/lightning/backend/tidb.go index 4e33d58f9..8cdc0c872 100644 --- a/pkg/lightning/backend/tidb.go +++ b/pkg/lightning/backend/tidb.go @@ -87,8 +87,7 @@ func NewTiDBBackend(db *sql.DB, onDuplicate string) Backend { func (row tidbRow) ClassifyAndAppend(data *Rows, checksum *verification.KVChecksum, _ *Rows, _ *verification.KVChecksum) { rows := (*data).(tidbRows) - // I'm not sure if `rows := data.(*tidbRows); *rows = append(*rows, row)` could solve this lint. - // Seems there are some stories. Leave it untouched. + // We cannot do `rows := data.(*tidbRows); *rows = append(*rows, row)`. //nolint:gocritic *data = append(rows, row) cs := verification.MakeKVChecksum(uint64(len(row)), 1, 0) diff --git a/pkg/lightning/mydump/region.go b/pkg/lightning/mydump/region.go index bca973781..9b4634fba 100644 --- a/pkg/lightning/mydump/region.go +++ b/pkg/lightning/mydump/region.go @@ -298,8 +298,10 @@ func makeParquetFileRegion( if err != nil { return prevRowIdxMax, nil, errors.Trace(err) } - // TODO: handle the error here? - numberRows, _ := ReadParquetFileRowCount(ctx, store, r, dataFile.FileMeta.Path) + 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, diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index e0df2c43d..d1a85c54a 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -801,7 +801,7 @@ func (rc *Controller) listenCheckpointUpdates() { lock.Unlock() - //nolint:scopelint // This would be either INLINED or ERASED, believe me! + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfImportedChunk", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { rc.checkpointsWg.Done() @@ -810,7 +810,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) - //nolint:scopelint // This would be either INLINED or ERASED, believe me! + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfStatusBecomes", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && merger.EngineID >= 0 && int(merger.Status) == val.(int) { rc.checkpointsWg.Done() @@ -819,7 +819,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) - //nolint:scopelint // This would be either INLINED or ERASED, believe me! + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("FailIfIndexEngineImported", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.StatusCheckpointMerger); ok && merger.EngineID == checkpoints.WholeTableEngineID && @@ -830,7 +830,7 @@ func (rc *Controller) listenCheckpointUpdates() { } }) - //nolint:scopelint // This would be either INLINED or ERASED, believe me! + //nolint:scopelint // This would be either INLINED or ERASED, at compile time. failpoint.Inject("KillIfImportedChunk", func(val failpoint.Value) { if merger, ok := scp.merger.(*checkpoints.ChunkCheckpointMerger); ok && merger.Checksum.SumKVS() >= uint64(val.(int)) { if err := common.KillMySelf(); err != nil { From 80b8c18e0a5128335100141b3b0fdbd257af39d9 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 9 Mar 2021 13:15:22 +0800 Subject: [PATCH 20/29] lightning: add nolint:interfacer for TestClose --- pkg/lightning/backend/backend_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/lightning/backend/backend_test.go b/pkg/lightning/backend/backend_test.go index 725f5b225..b8301608b 100644 --- a/pkg/lightning/backend/backend_test.go +++ b/pkg/lightning/backend/backend_test.go @@ -2,6 +2,7 @@ package backend_test import ( "context" + "gopkg.in/check.v1" "time" "github.com/golang/mock/gomock" @@ -273,7 +274,8 @@ func (s *backendSuite) TestImportFailedRecovered(c *C) { c.Assert(err, IsNil) } -func (s *backendSuite) TestClose(c gomock.TestReporter) { +//nolint:interfacer // change test case signature causes check panicking. +func (s *backendSuite) TestClose(c *check.C) { s.setUpTest(c) defer s.tearDownTest() From 90cc82cbc9e92298519c19cf3673c71a685ba71d Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 9 Mar 2021 13:47:50 +0800 Subject: [PATCH 21/29] go.sum: run go mod tidy --- go.mod1 | 1 + 1 file changed, 1 insertion(+) diff --git a/go.mod1 b/go.mod1 index 78a04e01c..d9446d167 100644 --- a/go.mod1 +++ b/go.mod1 @@ -50,6 +50,7 @@ require ( golang.org/x/text v0.3.5 google.golang.org/api v0.22.0 google.golang.org/grpc v1.27.1 + gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b modernc.org/mathutil v1.1.1 sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0 ) From 69b5f967ae4653a0e7e6813a76115ae293d90eb0 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 9 Mar 2021 14:26:07 +0800 Subject: [PATCH 22/29] go.sum: run go mod tidy --- go.mod1 | 1 - pkg/lightning/backend/backend_test.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/go.mod1 b/go.mod1 index d9446d167..78a04e01c 100644 --- a/go.mod1 +++ b/go.mod1 @@ -50,7 +50,6 @@ require ( golang.org/x/text v0.3.5 google.golang.org/api v0.22.0 google.golang.org/grpc v1.27.1 - gopkg.in/check.v1 v1.0.0-20200902074654-038fdea0a05b modernc.org/mathutil v1.1.1 sourcegraph.com/sourcegraph/appdash v0.0.0-20190731080439-ebfcffb1b5c0 ) diff --git a/pkg/lightning/backend/backend_test.go b/pkg/lightning/backend/backend_test.go index b8301608b..014fe1c8e 100644 --- a/pkg/lightning/backend/backend_test.go +++ b/pkg/lightning/backend/backend_test.go @@ -2,7 +2,6 @@ package backend_test import ( "context" - "gopkg.in/check.v1" "time" "github.com/golang/mock/gomock" @@ -275,7 +274,7 @@ func (s *backendSuite) TestImportFailedRecovered(c *C) { } //nolint:interfacer // change test case signature causes check panicking. -func (s *backendSuite) TestClose(c *check.C) { +func (s *backendSuite) TestClose(c *C) { s.setUpTest(c) defer s.tearDownTest() From 3cdd59eee4019793dbd9536f97f9e7dd36f0fd8d Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 9 Mar 2021 14:53:14 +0800 Subject: [PATCH 23/29] lightning: address comments --- pkg/lightning/backend/session.go | 7 +++-- pkg/lightning/backend/sql2kv.go | 6 ++--- pkg/lightning/backend/tidb.go | 6 ++--- pkg/lightning/checkpoints/checkpoints.go | 28 ++++++++++---------- pkg/lightning/checkpoints/glue_checkpoint.go | 16 +++++------ pkg/lightning/common/version.go | 12 +++++---- pkg/lightning/common/version_test.go | 12 ++++----- 7 files changed, 43 insertions(+), 44 deletions(-) diff --git a/pkg/lightning/backend/session.go b/pkg/lightning/backend/session.go index b50e57b21..d3c3d361d 100644 --- a/pkg/lightning/backend/session.go +++ b/pkg/lightning/backend/session.go @@ -19,10 +19,6 @@ import ( "fmt" "strconv" - "go.uber.org/zap" - - "github.com/pingcap/br/pkg/lightning/log" - "github.com/pingcap/parser/model" "github.com/pingcap/parser/mysql" "github.com/pingcap/tidb/kv" @@ -30,6 +26,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. diff --git a/pkg/lightning/backend/sql2kv.go b/pkg/lightning/backend/sql2kv.go index 951ee5b2f..3bfbf2fee 100644 --- a/pkg/lightning/backend/sql2kv.go +++ b/pkg/lightning/backend/sql2kv.go @@ -338,12 +338,12 @@ func (kvcodec *tableKVEncoder) Encode( if isAutoRandom && isPk { incrementalBits := autoRandomIncrementBits(col, int(kvcodec.tbl.Meta().AutoRandomBits)) if err := kvcodec.tbl.RebaseAutoID(kvcodec.se, value.GetInt64()&((1< Date: Tue, 9 Mar 2021 15:11:09 +0800 Subject: [PATCH 24/29] test: added a check to encode with _tidb_rows --- pkg/lightning/backend/tidb_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/lightning/backend/tidb_test.go b/pkg/lightning/backend/tidb_test.go index 93f77554d..942ffa01d 100644 --- a/pkg/lightning/backend/tidb_test.go +++ b/pkg/lightning/backend/tidb_test.go @@ -148,12 +148,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) - // TODO: check row here? - _, 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) { From 2c789ed68cbca342e762657578652bc196d1e068 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 16 Mar 2021 10:44:01 +0800 Subject: [PATCH 25/29] lightning/restore: rollback a refactor --- pkg/lightning/restore/restore.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/lightning/restore/restore.go b/pkg/lightning/restore/restore.go index 3de1bea44..27ff07321 100644 --- a/pkg/lightning/restore/restore.go +++ b/pkg/lightning/restore/restore.go @@ -76,11 +76,11 @@ const ( // DeliverPauser is a shared pauser to pause progress to (*chunkRestore).encodeLoop var DeliverPauser = common.NewPauser() -func getMinDeliverBytes() uint64 { +// nolint:gochecknoinits // TODO: refactor +func init() { failpoint.Inject("SetMinDeliverBytes", func(v failpoint.Value) { - failpoint.Return(uint64(v.(int))) + minDeliverBytes = uint64(v.(int)) }) - return 96 * units.KiB } type saveCp struct { @@ -2247,8 +2247,8 @@ func (tr *TableRestore) analyzeTable(ctx context.Context, g glue.SQLExecutor) er } var ( - maxKVQueueSize = 128 // Cache at most this number of rows before blocking the encode loop - minDeliverBytes uint64 = getMinDeliverBytes() // 96 KB (data + index). batch at least this amount of bytes to reduce number of messages + 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 ) type deliveredKVs struct { From 330ed2f61227f33168eaddac1645b64160ddf612 Mon Sep 17 00:00:00 2001 From: hillium Date: Wed, 17 Mar 2021 16:46:46 +0800 Subject: [PATCH 26/29] lightning: fix some new lints --- pkg/lightning/backend/importer/importer.go | 8 ++++---- pkg/lightning/backend/kv/sql2kv.go | 1 + pkg/lightning/backend/local/local.go | 20 ++++++++++---------- pkg/lightning/backend/tidb/tidb.go | 8 ++++---- pkg/lightning/tikv/tikv_test.go | 3 ++- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/pkg/lightning/backend/importer/importer.go b/pkg/lightning/backend/importer/importer.go index eb0cdb6a7..6c607b9ee 100644 --- a/pkg/lightning/backend/importer/importer.go +++ b/pkg/lightning/backend/importer/importer.go @@ -323,18 +323,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/sql2kv.go b/pkg/lightning/backend/kv/sql2kv.go index 0c94735d1..2f66834c2 100644 --- a/pkg/lightning/backend/kv/sql2kv.go +++ b/pkg/lightning/backend/kv/sql2kv.go @@ -283,6 +283,7 @@ 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)) } diff --git a/pkg/lightning/backend/local/local.go b/pkg/lightning/backend/local/local.go index b1ba02947..a3f30b394 100644 --- a/pkg/lightning/backend/local/local.go +++ b/pkg/lightning/backend/local/local.go @@ -214,7 +214,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()) @@ -259,7 +259,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 { @@ -1494,8 +1494,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), @@ -1793,7 +1793,7 @@ func (local *local) EngineFileSizes() (res []backend.EngineFileSize) { return } -type LocalWriter struct { +type Writer struct { writeErr common.OnceError local *File consumeCh chan struct{} @@ -1806,7 +1806,7 @@ type LocalWriter struct { writer *sstWriter } -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 @@ -1819,7 +1819,7 @@ func (w *LocalWriter) AppendRows(ctx context.Context, tableName string, columnNa return nil } -func (w *LocalWriter) Close() error { +func (w *Writer) Close() error { w.local.localWriters.Delete(w) close(w.kvsChan) @@ -1841,11 +1841,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() @@ -1910,7 +1910,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/tidb/tidb.go b/pkg/lightning/backend/tidb/tidb.go index 32fdd47ba..95102a619 100644 --- a/pkg/lightning/backend/tidb/tidb.go +++ b/pkg/lightning/backend/tidb/tidb.go @@ -583,18 +583,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/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() From 099b5ba19e2b0bc296083d95e6aa5285102beabd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B1=B1=E5=B2=9A?= <36239017+YuJuncen@users.noreply.github.com> Date: Mon, 29 Mar 2021 16:02:36 +0800 Subject: [PATCH 27/29] Apply suggestions from code review Co-authored-by: Neil Shen --- pkg/lightning/log/log.go | 2 +- pkg/lightning/mydump/parser.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/lightning/log/log.go b/pkg/lightning/log/log.go index fdf2796fb..50d938412 100644 --- a/pkg/lightning/log/log.go +++ b/pkg/lightning/log/log.go @@ -76,7 +76,7 @@ var ( // InitLogger initializes Lightning's and also the TiDB library's loggers. func InitLogger(cfg *Config, tidbLoglevel string) error { if err := logutil.InitLogger(&logutil.LogConfig{Config: pclog.Config{Level: tidbLoglevel}}); err != nil { - return err + return errors.Trace(err) } logCfg := &pclog.Config{ diff --git a/pkg/lightning/mydump/parser.go b/pkg/lightning/mydump/parser.go index e5275c12d..18079199e 100644 --- a/pkg/lightning/mydump/parser.go +++ b/pkg/lightning/mydump/parser.go @@ -527,8 +527,8 @@ 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. + // 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]) } From 6dd05e7a646c502c22e4c7ec8ac939edb31ff2db Mon Sep 17 00:00:00 2001 From: hillium Date: Fri, 2 Apr 2021 16:51:15 +0800 Subject: [PATCH 28/29] *: fix some new linter error --- pkg/lightning/backend/local/local.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lightning/backend/local/local.go b/pkg/lightning/backend/local/local.go index a75be61c1..2e6f81d0c 100644 --- a/pkg/lightning/backend/local/local.go +++ b/pkg/lightning/backend/local/local.go @@ -1472,7 +1472,7 @@ func (local *local) CheckRequirements(ctx context.Context, checkCtx *backend.Che return checkTiFlashVersion(ctx, local.g, checkCtx, *tidbVersion) } -func checkTiDBVersion(ctx context.Context, versionStr string, requiredMinVersion, requiredMaxVersion semver.Version) error { +func checkTiDBVersion(_ context.Context, versionStr string, requiredMinVersion, requiredMaxVersion semver.Version) error { return version.CheckTiDBVersion(versionStr, requiredMinVersion, requiredMaxVersion) } From 9baf6ea703b5593dcf746d53cba8f324c391b6b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=B1=B1=E5=B2=9A?= <36239017+YuJuncen@users.noreply.github.com> Date: Wed, 7 Apr 2021 12:36:45 +0800 Subject: [PATCH 29/29] Update pkg/lightning/checkpoints/checkpoints.go Co-authored-by: glorv --- pkg/lightning/checkpoints/checkpoints.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/lightning/checkpoints/checkpoints.go b/pkg/lightning/checkpoints/checkpoints.go index a9af7facc..6d7f05469 100644 --- a/pkg/lightning/checkpoints/checkpoints.go +++ b/pkg/lightning/checkpoints/checkpoints.go @@ -535,7 +535,7 @@ func IsCheckpointsDBExists(ctx context.Context, cfg *config.Config) (bool, error defer rows.Close() result := rows.Next() if err := rows.Err(); err != nil { - return false, err + return false, errors.Trace(err) } return result, nil