Merge branch 'master' into expression-testify

.github/licenserc.yml

@@ -8,6 +8,7 @@ header:
     - '.gitignore'
     - '.gitattributes'
     - '.golangci.yml'
+    - '.golangci_br.yml'
     - 'LICENSES/'
     - '**/*.md'
     - '**/*.json'

.golangci_br.yml

@@ -0,0 +1,83 @@
+run:
+  concurrency: 4
+  timeout: 7m
+
+output:
+  format: colored-line-number
+  print-issued-lines: true
+  print-linter-name: true
+
+linters:
+  enable-all: true
+  disable:
+    - gochecknoglobals
+    - gci
+    - wsl
+    - funlen
+    - gocognit
+    - godox
+    - gomnd
+    - testpackage
+    - nestif
+    - goerr113
+    - lll
+    - paralleltest
+    - nlreturn
+    - exhaustivestruct
+    - exhaustive
+    - godot
+    - gosec
+    - errorlint
+    - wrapcheck
+    - gomoddirectives
+    - bodyclose
+    - unused
+    - unparam
+    - wastedassign
+    - tagliatelle
+    - thelper
+    - nolintlint
+    - ineffassign
+    - nilerr
+    - noctx
+    - rowserrcheck
+    - predeclared
+    - ifshort
+    - cyclop
+    - whitespace
+    - unconvert
+    - forcetypeassert
+    - sqlclosecheck
+    - errcheck
+    - gocritic
+    - golint
+    - gocyclo
+    - promlinter
+    - forbidigo
+    - gochecknoinits
+    - scopelint
+    - revive
+    - misspell
+    - maligned
+    - makezero
+    - interfacer
+    - gofumpt
+    - goconst
+    - prealloc
+    - govet
+    - dupl
+    - deadcode
+    - varcheck
+    - nakedret
+
+linters-settings:
+  staticcheck:
+    checks: ["S1002","S1004","S1007","S1009","S1010","S1012","S1019","S1020","S1021","S1024","S1030","SA2*","SA3*","SA4009","SA5*","SA6000","SA6001","SA6005", "-SA2002"]
+  stylecheck:
+    checks: ["-ST1003"]
+  gosec:
+    excludes:
+      - G601
+
+issues:
+  exclude-rules:

Makefile

@@ -44,7 +44,8 @@ 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_TIDB_TESTS))
+	GO111MODULE=on CGO_ENABLED=0 tools/bin/golangci-lint run -v $$($(PACKAGE_DIRECTORIES_TIDB_TESTS)) --config .golangci.yml
+	GO111MODULE=on CGO_ENABLED=0 tools/bin/golangci-lint run -v $$($(BR_PACKAGE_DIRECTORIES)) --config .golangci_br.yml
 
 unconvert:tools/bin/unconvert
 	@echo "unconvert check(skip check the genenrated or copied code in lightning)"
@@ -386,4 +387,4 @@ dumpling_bins:
 	@which bin/tidb-server
 	@which bin/minio
 	@which bin/tidb-lightning
-	@which bin/sync_diff_inspector
+	@which bin/sync_diff_inspector

Makefile.common

@@ -44,6 +44,7 @@ endif
 ARCH      := "`uname -s`"
 LINUX     := "Linux"
 MAC       := "Darwin"
+
 PACKAGE_LIST  := go list ./...
 PACKAGE_LIST_TIDB_TESTS  := go list ./... | grep -vE "github.com\/pingcap\/tidb\/br|github.com\/pingcap\/tidb\/cmd|github.com\/pingcap\/tidb\/dumpling"
 PACKAGES  ?= $$($(PACKAGE_LIST))
@@ -52,6 +53,7 @@ PACKAGE_DIRECTORIES := $(PACKAGE_LIST) | 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_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))
 
@@ -88,6 +90,7 @@ endif
 
 BR_PKG := github.com/pingcap/tidb/br
 BR_PACKAGES       := go list ./...| grep "github.com\/pingcap\/tidb\/br"
+BR_PACKAGE_DIRECTORIES := $(BR_PACKAGES) | sed 's|github.com/pingcap/$(PROJECT)/||'
 LIGHTNING_BIN     := bin/tidb-lightning
 LIGHTNING_CTL_BIN := bin/tidb-lightning-ctl
 BR_BIN            := bin/br

br/cmd/tidb-lightning/main.go

@@ -88,7 +88,6 @@ func main() {
 
 	if err != nil {
 		logger.Error("tidb lightning encountered error stack info", zap.Error(err))
-		logger.Error("tidb lightning encountered error", log.ShortError(err))
 		fmt.Fprintln(os.Stderr, "tidb lightning encountered error: ", err)
 	} else {
 		logger.Info("tidb lightning exit")

br/pkg/lightning/backend/tidb/tidb.go

@@ -681,8 +681,7 @@ func (be *tidbBackend) LocalWriter(
 }
 
 type Writer struct {
-	be       *tidbBackend
-	errorMgr *errormanager.ErrorManager
+	be *tidbBackend
 }
 
 func (w *Writer) Close(ctx context.Context) (backend.ChunkFlushStatus, error) {

br/pkg/lightning/restore/check_template.go

@@ -16,6 +16,7 @@ package restore
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/jedib0t/go-pretty/v6/table"
 	"github.com/jedib0t/go-pretty/v6/text"
@@ -41,12 +42,16 @@ type Template interface {
 
 	// Output print all checks results.
 	Output() string
+
+	// FailedMsg represents the error msg for the failed check.
+	FailedMsg() string
 }
 
 type SimpleTemplate struct {
 	count               int
 	warnFailedCount     int
 	criticalFailedCount int
+	failedMsg           []string
 	t                   table.Writer
 }
 
@@ -63,10 +68,15 @@ func NewSimpleTemplate() Template {
 		0,
 		0,
 		0,
+		make([]string, 0),
 		t,
 	}
 }
 
+func (c *SimpleTemplate) FailedMsg() string {
+	return strings.Join(c.failedMsg, ";\n")
+}
+
 func (c *SimpleTemplate) Collect(t CheckType, passed bool, msg string) {
 	c.count++
 	if !passed {
@@ -77,6 +87,7 @@ func (c *SimpleTemplate) Collect(t CheckType, passed bool, msg string) {
 			c.warnFailedCount++
 		}
 	}
+	c.failedMsg = append(c.failedMsg, msg)
 	c.t.AppendRow(table.Row{c.count, msg, t, passed})
 	c.t.AppendSeparator()
 }

br/pkg/lightning/restore/restore.go

@@ -1804,13 +1804,13 @@ func (rc *Controller) preCheckRequirements(ctx context.Context) error {
 
 	if rc.tidbGlue.OwnsSQLExecutor() && rc.cfg.App.CheckRequirements {
 		fmt.Print(rc.checkTemplate.Output())
-		if !rc.checkTemplate.Success() {
-			if !taskExist && rc.taskMgr != nil {
-				rc.taskMgr.CleanupTask(ctx)
-			}
-			return errors.Errorf("tidb-lightning pre-check failed." +
-				" Please fix the failed check(s) or set --check-requirements=false to skip checks")
+	}
+	if !rc.checkTemplate.Success() {
+		if !taskExist && rc.taskMgr != nil {
+			rc.taskMgr.CleanupTask(ctx)
 		}
+		return errors.Errorf("tidb-lightning check failed."+
+			" Please fix the failed check(s):\n %s", rc.checkTemplate.FailedMsg())
 	}
 	return nil
 }

br/pkg/pdutil/pd.go

@@ -118,14 +118,13 @@ var (
 	}
 
 	// defaultPDCfg find by https://github.com/tikv/pd/blob/master/conf/config.toml.
+	// only use for debug command.
 	defaultPDCfg = map[string]interface{}{
 		"max-merge-region-keys":       200000,
 		"max-merge-region-size":       20,
 		"leader-schedule-limit":       4,
 		"region-schedule-limit":       2048,
-		"max-snapshot-count":          3,
 		"enable-location-replacement": "true",
-		"max-pending-peer-count":      16,
 	}
 )
 

br/tests/br_other/run.sh

@@ -135,9 +135,7 @@ default_pd_values='{
   "max-merge-region-keys": 200000,
   "max-merge-region-size": 20,
   "leader-schedule-limit": 4,
-  "region-schedule-limit": 2048,
-  "max-snapshot-count":    3,
-  "max-pending-peer-count": 16
+  "region-schedule-limit": 2048
 }'
 
 for key in $(echo $default_pd_values | jq 'keys[]'); do

br/tests/lightning_checkpoint_dirty_tableid/run.sh

@@ -35,8 +35,12 @@ set -e
 ILLEGAL_CP_COUNT=$(grep "TiDB Lightning has detected tables with illegal checkpoints. To prevent data loss, this run will stop now." "$TEST_DIR/lightning-checkpoint-dirty-tableid.log" | wc -l)
 TABLE_SUGGEST=$(grep "checkpoint-remove=" "$TEST_DIR/lightning-checkpoint-dirty-tableid.log" | wc -l)
 
-[ $ILLEGAL_CP_COUNT -eq 1 ]
-[ $TABLE_SUGGEST -eq 1 ]
+# we got same errors in three place:
+# 1. run failed in step 2
+# 2. the whole procedure failed
+# 3. main
+[ $ILLEGAL_CP_COUNT -eq 3 ]
+[ $TABLE_SUGGEST -eq 3 ]
 
 # Try again with the file checkpoints
 
@@ -60,5 +64,9 @@ set -e
 ILLEGAL_CP_COUNT=$(grep "TiDB Lightning has detected tables with illegal checkpoints. To prevent data loss, this run will stop now." "$TEST_DIR/lightning-checkpoint-dirty-tableid.log" | wc -l)
 TABLE_SUGGEST=$(grep "checkpoint-remove=" "$TEST_DIR/lightning-checkpoint-dirty-tableid.log" | wc -l)
 
-[ $ILLEGAL_CP_COUNT -eq 1 ]
-[ $TABLE_SUGGEST -eq 1 ]
+# we got same errors in three place:
+# 1. run failed in step 2
+# 2. the whole procedure failed
+# 3. main
+[ $ILLEGAL_CP_COUNT -eq 3 ]
+[ $TABLE_SUGGEST -eq 3 ]

br/tests/lightning_duplicate_detection/run.sh

@@ -16,6 +16,9 @@
 
 set -eux
 
+# skip unstable test for temporary
+exit 0
+
 check_cluster_version 5 2 0 'duplicate detection' || exit 0
 
 LOG_FILE1="$TEST_DIR/lightning-duplicate-detection1.log"

br/tests/lightning_error_summary/run.sh

@@ -16,6 +16,8 @@
 
 set -eux
 
+# skip for temporary due to checksum for table a,c succeed, but expect to fail.
+exit 0
 # Check that error summary are written at the bottom of import.
 run_sql 'DROP DATABASE IF EXISTS tidb_lightning_checkpoint_error_summary;'
 

br/tests/lightning_examples/run.sh

@@ -16,7 +16,7 @@
 
 set -eu
 
-EXAMPLES_PATH=${EXAMPLES_PATH:-pkg/lightning/mydump/examples}
+EXAMPLES_PATH=${EXAMPLES_PATH:-br/pkg/lightning/mydump/examples}
 
 # Because of issue JENKINS-45544 we can't use the Unicode filename in the
 # examples. We are going to rename it in-place.

parser/consistent_test.go

@@ -20,58 +20,48 @@ import (
 	"runtime"
 	"sort"
 	"strings"
+	"testing"
 
-	. "github.com/pingcap/check"
+	requires "github.com/stretchr/testify/require"
 )
 
-var _ = Suite(&testConsistentSuite{})
+func TestKeywordConsistent(t *testing.T) {
+	t.Parallel()
 
-type testConsistentSuite struct {
-	content string
-
-	reservedKeywords   []string
-	unreservedKeywords []string
-	notKeywordTokens   []string
-	tidbKeywords       []string
-}
-
-func (s *testConsistentSuite) SetUpSuite(c *C) {
 	_, filename, _, _ := runtime.Caller(0)
 	parserFilename := path.Join(path.Dir(filename), "parser.y")
 	parserFile, err := os.Open(parserFilename)
-	c.Assert(err, IsNil)
+	requires.NoError(t, err)
 	data, err := ioutil.ReadAll(parserFile)
-	c.Assert(err, IsNil)
-	s.content = string(data)
+	requires.NoError(t, err)
+	content := string(data)
 
 	reservedKeywordStartMarker := "\t/* The following tokens belong to ReservedKeyword. Notice: make sure these tokens are contained in ReservedKeyword. */"
 	unreservedKeywordStartMarker := "\t/* The following tokens belong to UnReservedKeyword. Notice: make sure these tokens are contained in UnReservedKeyword. */"
 	notKeywordTokenStartMarker := "\t/* The following tokens belong to NotKeywordToken. Notice: make sure these tokens are contained in NotKeywordToken. */"
 	tidbKeywordStartMarker := "\t/* The following tokens belong to TiDBKeyword. Notice: make sure these tokens are contained in TiDBKeyword. */"
 	identTokenEndMarker := "%token\t<item>"
 
-	s.reservedKeywords = extractKeywords(s.content, reservedKeywordStartMarker, unreservedKeywordStartMarker)
-	s.unreservedKeywords = extractKeywords(s.content, unreservedKeywordStartMarker, notKeywordTokenStartMarker)
-	s.notKeywordTokens = extractKeywords(s.content, notKeywordTokenStartMarker, tidbKeywordStartMarker)
-	s.tidbKeywords = extractKeywords(s.content, tidbKeywordStartMarker, identTokenEndMarker)
-}
+	reservedKeywords := extractKeywords(content, reservedKeywordStartMarker, unreservedKeywordStartMarker)
+	unreservedKeywords := extractKeywords(content, unreservedKeywordStartMarker, notKeywordTokenStartMarker)
+	notKeywordTokens := extractKeywords(content, notKeywordTokenStartMarker, tidbKeywordStartMarker)
+	tidbKeywords := extractKeywords(content, tidbKeywordStartMarker, identTokenEndMarker)
 
-func (s *testConsistentSuite) TestKeywordConsistent(c *C) {
 	for k, v := range aliases {
-		c.Assert(k, Not(Equals), v)
-		c.Assert(tokenMap[k], Equals, tokenMap[v])
+		requires.NotEqual(t, k, v)
+		requires.Equal(t, tokenMap[v], tokenMap[k])
 	}
-	keywordCount := len(s.reservedKeywords) + len(s.unreservedKeywords) + len(s.notKeywordTokens) + len(s.tidbKeywords)
-	c.Assert(len(tokenMap)-len(aliases), Equals, keywordCount-len(windowFuncTokenMap))
+	keywordCount := len(reservedKeywords) + len(unreservedKeywords) + len(notKeywordTokens) + len(tidbKeywords)
+	requires.Equal(t, keywordCount-len(windowFuncTokenMap), len(tokenMap)-len(aliases))
 
-	unreservedCollectionDef := extractKeywordsFromCollectionDef(s.content, "\nUnReservedKeyword:")
-	c.Assert(s.unreservedKeywords, DeepEquals, unreservedCollectionDef)
+	unreservedCollectionDef := extractKeywordsFromCollectionDef(content, "\nUnReservedKeyword:")
+	requires.Equal(t, unreservedCollectionDef, unreservedKeywords)
 
-	notKeywordTokensCollectionDef := extractKeywordsFromCollectionDef(s.content, "\nNotKeywordToken:")
-	c.Assert(s.notKeywordTokens, DeepEquals, notKeywordTokensCollectionDef)
+	notKeywordTokensCollectionDef := extractKeywordsFromCollectionDef(content, "\nNotKeywordToken:")
+	requires.Equal(t, notKeywordTokensCollectionDef, notKeywordTokens)
 
-	tidbKeywordsCollectionDef := extractKeywordsFromCollectionDef(s.content, "\nTiDBKeyword:")
-	c.Assert(s.tidbKeywords, DeepEquals, tidbKeywordsCollectionDef)
+	tidbKeywordsCollectionDef := extractKeywordsFromCollectionDef(content, "\nTiDBKeyword:")
+	requires.Equal(t, tidbKeywordsCollectionDef, tidbKeywords)
 }
 
 func extractMiddle(str, startMarker, endMarker string) string {