Merge branch 'master' of https://github.com/fengou1/tidb into br_ddl

.golangci.yml

@@ -18,6 +18,7 @@ linters:
     - gosec
     - asciicheck
     - bodyclose
+    - rowserrcheck
 linters-settings:
   staticcheck:
     checks: ["S1002","S1004","S1007","S1009","S1010","S1012","S1019","S1020","S1021","S1024","S1030","SA2*","SA3*","SA4009","SA5*","SA6000","SA6001","SA6005", "-SA2002"]
@@ -32,3 +33,4 @@ issues:
       linters:
         - errcheck
         - gosec
+        - rowserrcheck

Makefile

@@ -44,7 +44,7 @@ goword:tools/bin/goword
 	tools/bin/goword $(FILES) 2>&1 | $(FAIL_ON_STDOUT)
 
 check-static: tools/bin/golangci-lint
-	tools/bin/golangci-lint run -v $$($(PACKAGE_DIRECTORIES_WITHOUT_BR))
+	tools/bin/golangci-lint run -v $$($(PACKAGE_DIRECTORIES_TIDB_TESTS))
 
 unconvert:tools/bin/unconvert
 	@echo "unconvert check(skip check the genenrated or copied code in lightning)"
@@ -60,11 +60,11 @@ errdoc:tools/bin/errdoc-gen
 
 lint:tools/bin/revive
 	@echo "linting"
-	@tools/bin/revive -formatter friendly -config tools/check/revive.toml $(FILES_WITHOUT_BR)
+	@tools/bin/revive -formatter friendly -config tools/check/revive.toml $(FILES_TIDB_TESTS)
 
 vet:
 	@echo "vet"
-	$(GO) vet -all $(PACKAGES_WITHOUT_BR) 2>&1 | $(FAIL_ON_STDOUT)
+	$(GO) vet -all $(PACKAGES_TIDB_TESTS) 2>&1 | $(FAIL_ON_STDOUT)
 
 tidy:
 	@echo "go mod tidy"
@@ -114,44 +114,20 @@ ifeq ("$(TRAVIS_COVERAGE)", "1")
 endif
 
 devgotest: failpoint-enable
-ifeq ("$(TRAVIS_COVERAGE)", "1")
-	@echo "Running in TRAVIS_COVERAGE mode."
-	$(GO) get github.com/go-playground/overalls
-	@export log_level=info; \
-	$(OVERALLS) -project=github.com/pingcap/tidb \
-			-covermode=count \
-			-ignore='.git,br,vendor,cmd,docs,tests,LICENSES' \
-			-concurrency=4 \
-			-- -coverpkg=./... \
-			|| { $(FAILPOINT_DISABLE); exit 1; }
-else
 # grep regex: Filter out all tidb logs starting with:
 # - '[20' (like [2021/09/15 ...] [INFO]..)
 # - 'PASS:' to ignore passed tests
 # - 'ok ' to ignore passed directories
 	@echo "Running in native mode."
 	@export log_level=info; export TZ='Asia/Shanghai'; \
-	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_WITHOUT_BR) -check.p true > gotest.log || { $(FAILPOINT_DISABLE); grep -v '^\([[]20\|PASS:\|ok \)' 'gotest.log'; exit 1; }
-endif
+	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_TIDB_TESTS) -check.p true > gotest.log || { $(FAILPOINT_DISABLE); grep -v '^\([[]20\|PASS:\|ok \)' 'gotest.log'; exit 1; }
 	@$(FAILPOINT_DISABLE)
 
 
 gotest: failpoint-enable
-ifeq ("$(TRAVIS_COVERAGE)", "1")
-	@echo "Running in TRAVIS_COVERAGE mode."
-	$(GO) get github.com/go-playground/overalls
-	@export log_level=info; \
-	$(OVERALLS) -project=github.com/pingcap/tidb \
-			-covermode=count \
-			-ignore='.git,br,vendor,cmd,docs,tests,LICENSES' \
-			-concurrency=4 \
-			-- -coverpkg=./... \
-			|| { $(FAILPOINT_DISABLE); exit 1; }
-else
 	@echo "Running in native mode."
 	@export log_level=info; export TZ='Asia/Shanghai'; \
-	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_WITHOUT_BR) -check.p true > gotest.log || { $(FAILPOINT_DISABLE); cat 'gotest.log'; exit 1; }
-endif
+	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_TIDB_TESTS) -check.p true > gotest.log || { $(FAILPOINT_DISABLE); cat 'gotest.log'; exit 1; }
 	@$(FAILPOINT_DISABLE)
 
 race: failpoint-enable

Makefile.common

@@ -44,14 +44,14 @@ endif
 ARCH      := "`uname -s`"
 LINUX     := "Linux"
 MAC       := "Darwin"
-PACKAGE_LIST  := go list ./...| grep -vE "cmd|github.com\/pingcap\/tidb\/tests"
-PACKAGE_LIST_WITHOUT_BR  := go list ./...| grep -vE "cmd|github.com\/pingcap\/tidb\/tests|github.com\/pingcap\/tidb\/br"
+PACKAGE_LIST  := go list ./...
+PACKAGE_LIST_TIDB_TESTS  := go list ./... | grep -vE "github.com\/pingcap\/tidb\/br|github.com\/pingcap\/tidb\/cmd"
 PACKAGES  ?= $$($(PACKAGE_LIST))
-PACKAGES_WITHOUT_BR ?= $$($(PACKAGE_LIST_WITHOUT_BR))
+PACKAGES_TIDB_TESTS ?= $$($(PACKAGE_LIST_TIDB_TESTS))
 PACKAGE_DIRECTORIES := $(PACKAGE_LIST) | sed 's|github.com/pingcap/$(PROJECT)/||'
-PACKAGE_DIRECTORIES_WITHOUT_BR := $(PACKAGE_LIST_WITHOUT_BR) | sed 's|github.com/pingcap/$(PROJECT)/||'
+PACKAGE_DIRECTORIES_TIDB_TESTS := $(PACKAGE_LIST_TIDB_TESTS) | sed 's|github.com/pingcap/$(PROJECT)/||'
 FILES     := $$(find $$($(PACKAGE_DIRECTORIES)) -name "*.go")
-FILES_WITHOUT_BR := $$(find $$($(PACKAGE_DIRECTORIES_WITHOUT_BR)) -name "*.go")
+FILES_TIDB_TESTS := $$(find $$($(PACKAGE_DIRECTORIES_TIDB_TESTS)) -name "*.go")
 UNCONVERT_PACKAGES_LIST := go list ./...| grep -vE "lightning\/checkpoints|lightning\/manual|lightning\/common"
 UNCONVERT_PACKAGES := $$($(UNCONVERT_PACKAGES_LIST))
 

br/cmd/tidb-lightning-ctl/main.go

@@ -160,7 +160,6 @@ func fetchMode(ctx context.Context, cfg *config.Config, tls *common.TLS) error {
 	)
 }
 
-
 func checkpointErrorIgnore(ctx context.Context, cfg *config.Config, tableName string) error {
 	cpdb, err := checkpoints.OpenCheckpointsDB(ctx, cfg)
 	if err != nil {
@@ -350,4 +349,3 @@ func importEngine(ctx context.Context, cfg *config.Config, tls *common.TLS, engi
 	}
 	return errors.Trace(ce.Import(ctx, regionSplitSize))
 }
-

br/pkg/lightning/restore/meta_manager.go

@@ -234,6 +234,9 @@ func (m *dbTableMetaMgr) AllocTableRowIDs(ctx context.Context, rawRowIDMax int64
 				maxRowIDMax = rowIDMax
 			}
 		}
+		if rows.Err() != nil {
+			return errors.Trace(rows.Err())
+		}
 
 		// no enough info are available, fetch row_id max for table
 		if curStatus == metaStatusInitial {
@@ -441,6 +444,9 @@ func (m *dbTableMetaMgr) CheckAndUpdateLocalChecksum(ctx context.Context, checks
 		}
 		rows.Close()
 		closed = true
+		if rows.Err() != nil {
+			return errors.Trace(rows.Err())
+		}
 
 		query = fmt.Sprintf("update %s set total_kvs = ?, total_bytes = ?, checksum = ?, status = ?, has_duplicates = ? where table_id = ? and task_id = ?", m.tableName)
 		_, err = tx.ExecContext(ctx, query, checksum.SumKVS(), checksum.SumSize(), checksum.Sum(), newStatus.String(), hasLocalDupes, m.tr.tableInfo.ID, m.taskID)
@@ -602,8 +608,14 @@ func (m *dbTaskMetaMgr) CheckTaskExist(ctx context.Context) (bool, error) {
 				exist = true
 			}
 		}
-		err = rows.Close()
-		return errors.Trace(err)
+		if err := rows.Close(); err != nil {
+			return errors.Trace(err)
+		}
+		if err := rows.Err(); err != nil {
+			return errors.Trace(err)
+		}
+
+		return nil
 	})
 	return exist, errors.Trace(err)
 }
@@ -731,6 +743,10 @@ func (m *dbTaskMetaMgr) CheckAndPausePdSchedulers(ctx context.Context) (pdutil.U
 		}
 		closed = true
 
+		if err = rows.Err(); err != nil {
+			return errors.Trace(err)
+		}
+
 		if cfgStr != "" {
 			err = json.Unmarshal([]byte(cfgStr), &pausedCfg)
 			return errors.Trace(err)
@@ -852,6 +868,10 @@ func (m *dbTaskMetaMgr) CheckAndFinishRestore(ctx context.Context, finished bool
 		}
 		closed = true
 
+		if err = rows.Err(); err != nil {
+			return errors.Trace(err)
+		}
+
 		if taskStatus < taskMetaStatusSwitchSkipped {
 			newStatus := taskMetaStatusSwitchBack
 			newState := taskStateNormal

br/pkg/lightning/restore/restore.go

@@ -770,7 +770,12 @@ func (rc *Controller) restoreSchema(ctx context.Context) error {
 
 	go rc.listenCheckpointUpdates()
 
-	rc.sysVars = ObtainImportantVariables(ctx, rc.tidbGlue.GetSQLExecutor(), !rc.isTiDBBackend())
+	sysVars := ObtainImportantVariables(ctx, rc.tidbGlue.GetSQLExecutor(), !rc.isTiDBBackend())
+	// override by manually set vars
+	for k, v := range rc.cfg.TiDB.Vars {
+		sysVars[k] = v
+	}
+	rc.sysVars = sysVars
 
 	// Estimate the number of chunks for progress reporting
 	err = rc.estimateChunkCountIntoMetrics(ctx)

br/pkg/lightning/restore/restore_test.go

@@ -1553,8 +1553,23 @@ func (s *restoreSchemaSuite) TearDownTest(c *C) {
 }
 
 func (s *restoreSchemaSuite) TestRestoreSchemaSuccessful(c *C) {
+	// before restore, if sysVars is initialized by other test, the time_zone should be default value
+	if len(s.rc.sysVars) > 0 {
+		tz, ok := s.rc.sysVars["time_zone"]
+		c.Assert(ok, IsTrue)
+		c.Assert(tz, Equals, "SYSTEM")
+	}
+
+	s.rc.cfg.TiDB.Vars = map[string]string{
+		"time_zone": "UTC",
+	}
 	err := s.rc.restoreSchema(s.ctx)
 	c.Assert(err, IsNil)
+
+	// test after restore schema, sysVars has been updated
+	tz, ok := s.rc.sysVars["time_zone"]
+	c.Assert(ok, IsTrue)
+	c.Assert(tz, Equals, "UTC")
 }
 
 func (s *restoreSchemaSuite) TestRestoreSchemaFailed(c *C) {

br/pkg/lightning/restore/tidb.go

@@ -83,7 +83,6 @@ func isUnknownSystemVariableErr(err error) bool {
 }
 
 func DBFromConfig(ctx context.Context, dsn config.DBStore) (*sql.DB, error) {
-
 	param := common.MySQLConnectParam{
 		Host:             dsn.Host,
 		Port:             dsn.Port,
@@ -209,7 +208,7 @@ func createDatabaseIfNotExistStmt(dbName string) string {
 }
 
 func createTableIfNotExistsStmt(p *parser.Parser, createTable, dbName, tblName string) ([]string, error) {
-	stmts, _, err := p.Parse(createTable, "", "")
+	stmts, _, err := p.ParseSQL(createTable)
 	if err != nil {
 		return []string{}, err
 	}

br/pkg/utils/backoff.go

@@ -3,6 +3,9 @@
 package utils
 
 import (
+	"context"
+	"database/sql"
+	"io"
 	"time"
 
 	"github.com/pingcap/errors"
@@ -102,8 +105,27 @@ func NewPDReqBackoffer() Backoffer {
 }
 
 func (bo *pdReqBackoffer) NextBackoff(err error) time.Duration {
-	bo.delayTime = 2 * bo.delayTime
-	bo.attempt--
+	// bo.delayTime = 2 * bo.delayTime
+	// bo.attempt--
+	e := errors.Cause(err)
+	switch e { // nolint:errorlint
+	case nil, context.Canceled, context.DeadlineExceeded, io.EOF, sql.ErrNoRows:
+		// Excepted error, finish the operation
+		bo.delayTime = 0
+		bo.attempt = 0
+	default:
+		switch status.Code(e) {
+		case codes.DeadlineExceeded, codes.NotFound, codes.AlreadyExists, codes.PermissionDenied, codes.ResourceExhausted, codes.Aborted, codes.OutOfRange, codes.Unavailable, codes.DataLoss, codes.Unknown:
+			bo.delayTime = 2 * bo.delayTime
+			bo.attempt--
+		default:
+			// Unexcepted error
+			bo.delayTime = 0
+			bo.attempt = 0
+			log.Warn("unexcepted error, stop to retry", zap.Error(err))
+		}
+	}
+
 	if bo.delayTime > bo.maxDelayTime {
 		return bo.maxDelayTime
 	}

br/pkg/utils/retry.go

@@ -58,11 +58,6 @@ func WithRetry(
 		err := retryableFunc()
 		if err != nil {
 			allErrors = multierr.Append(allErrors, err)
-			retry := IsRetryableError(err)
-			if !retry { // exited retry
-				return allErrors
-			}
-
 			select {
 			case <-ctx.Done():
 				return allErrors // nolint:wrapcheck

cmd/benchdb/main.go

@@ -224,6 +224,7 @@ func (ut *benchDB) runCountTimes(name string, count int, f func()) {
 		name, sum/time.Duration(count), count, sum, first, last, max, min)
 }
 
+// #nosec G404
 func (ut *benchDB) insertRows(spec string) {
 	start, end, _ := ut.mustParseSpec(spec)
 	loopCount := (end - start + *batchSize - 1) / *batchSize
@@ -244,6 +245,7 @@ func (ut *benchDB) insertRows(spec string) {
 	})
 }
 
+// #nosec G404
 func (ut *benchDB) updateRandomRows(spec string) {
 	start, end, totalCount := ut.mustParseSpec(spec)
 	loopCount := (totalCount + *batchSize - 1) / *batchSize

cmd/benchfilesort/main.go

@@ -56,6 +56,7 @@ var (
 	outputRatio int
 )
 
+// #nosec G404
 func nextRow(r *rand.Rand, keySize int, valSize int) *comparableRow {
 	key := make([]types.Datum, keySize)
 	for i := range key {
@@ -188,6 +189,7 @@ func decodeMeta(fd *os.File) error {
  *		One 64-bit integer represent the row size in bytes, followed by the
  *      the actual row bytes.
  */
+// #nosec G404
 func export() error {
 	var outputBytes []byte
 

cmd/benchkv/main.go

@@ -14,6 +14,7 @@
 
 package main
 
+// #nosec G108
 import (
 	"context"
 	"flag"
@@ -129,7 +130,12 @@ func main() {
 	resp, err := http.Get("http://localhost:9191/metrics")
 	terror.MustNil(err)
 
-	defer terror.Call(resp.Body.Close)
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.Error("function call errored", zap.Error(err), zap.Stack("stack"))
+		}
+	}()
+
 	text, err1 := io.ReadAll(resp.Body)
 	terror.Log(errors.Trace(err1))
 

cmd/benchraw/main.go

@@ -14,6 +14,7 @@
 
 package main
 
+// #nosec G108
 import (
 	"context"
 	"flag"

cmd/ddltest/random.go

@@ -12,6 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// #nosec G404
+
 package ddltest
 
 import (