Merge remote-tracking branch 'upstream/master' into char-union-int-ca…

…st-char

.github/workflows/br_compatible_test.yml

@@ -2,6 +2,7 @@ name: BR / Compatibility Test
 
 on:
   push:
+  # merged git action
     branches:
       - master
       - 'release-[0-9].[0-9]*'
@@ -14,19 +15,20 @@ on:
       - '!br/docs/**'
       - '!br/tests/**'
       - '!br/docker/**'
-  pull_request:
-    branches:
-      - master
-      - 'release-[0-9].[0-9]*'
-    paths:
-      - 'br/**'
-      - '!**.html'
-      - '!**.md'
-      - '!CNAME'
-      - '!LICENSE'
-      - '!br/docs/**'
-      - '!br/tests/**'
-      - '!br/docker/**'
+  # disable pull request only keep the merge action since it is very costly to run those tests
+  # pull_request:
+  #   branches:
+  #     - master
+  #     - 'release-[0-9].[0-9]*'
+  #   paths:
+  #     - 'br/**'
+  #     - '!**.html'
+  #     - '!**.md'
+  #     - '!CNAME'
+  #     - '!LICENSE'
+  #     - '!br/docs/**'
+  #     - '!br/tests/**'
+  #     - '!br/docker/**'
 
 # See: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#concurrency.
 concurrency:

.github/workflows/compile_br.yaml

@@ -13,7 +13,10 @@ on:
       - '!br/docs/**'
       - '!br/tests/**'
       - '!br/docker/**'
+#change trigger policy
   pull_request:
+    types:
+      - labeled  # <--
     branches:
       - master
       - 'release-[0-9].[0-9]*'
@@ -35,8 +38,25 @@ concurrency:
   cancel-in-progress: true
 
 jobs:
+  compile-windows:
+    if: github.event_name == 'push' || github.event_name == 'pull_request' && github.event.label.name == 'action/run-br-cross-platform-build'
+    name: Compile for Windows job
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v2.1.0
+
+      - name: Set up Go
+        uses: actions/setup-go@v2
+        with:
+          go-version: 1.16
+
+      - name: Run build
+        run: make build_tools
+
   compile:
+    if: github.event_name == 'pull_request' && github.event.label.name == 'action/run-br-cross-platform-build'
     name: Compile for ${{ matrix.os }} / ${{ matrix.target}}
+
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
@@ -47,8 +67,6 @@ jobs:
           - os: ubuntu-latest
             target: aarch64-unknown-linux-gnu
 
-          - os: windows-latest
-            target: x86_64-pc-windows-msvc
     steps:
       - uses: actions/checkout@v2.1.0
 
@@ -61,6 +79,7 @@ jobs:
         run: make build_tools
 
   compile-freebsd:
+    if: github.event_name == 'pull_request' && github.event.label.name == 'action/run-br-cross-platform-build'
     name: Compile for FreeBSD job
     runs-on: ubuntu-latest
     steps:

.github/workflows/dumpling_integration_test.yml

@@ -27,6 +27,11 @@ on:
       - 'util/codec/**'
       - 'parser/model/**'
 
+# See: https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#concurrency.
+concurrency:
+  group: ${{ github.ref }}-${{ github.workflow }}
+  cancel-in-progress: true
+
 jobs:
   unit-test:
     runs-on: ubuntu-latest

.golangci.yml

@@ -21,18 +21,18 @@ linters:
     - exportloopref
     - rowserrcheck
     - unconvert
+    - makezero
+
 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:
     - path: _test\.go
       linters:
         - errcheck
         - gosec
         - rowserrcheck
+        - makezero

Makefile

@@ -109,11 +109,11 @@ explaintest: server_check
 ddltest:
 	@cd cmd/ddltest && $(GO) test -o ../../bin/ddltest -c
 
-upload-coverage: SHELL:=/bin/bash
 upload-coverage:
-ifeq ("$(TRAVIS_COVERAGE)", "1")
-	mv overalls.coverprofile coverage.txt
-	bash <(curl -s https://codecov.io/bash)
+ifneq ($(CODECOV_TOKEN), "")
+	curl -LO ${FILE_SERVER_URL}/download/cicd/ci-tools/codecov
+	chmod +x codecov
+	./codecov -t ${CODECOV_TOKEN}
 endif
 
 devgotest: failpoint-enable
@@ -129,7 +129,7 @@ devgotest: failpoint-enable
 gotest: failpoint-enable
 	@echo "Running in native mode."
 	@export log_level=info; export TZ='Asia/Shanghai'; \
-	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_TIDB_TESTS) -check.p true > gotest.log || { $(FAILPOINT_DISABLE); cat 'gotest.log'; exit 1; }
+	$(GOTEST) -ldflags '$(TEST_LDFLAGS)' $(EXTRA_TEST_ARGS) -cover $(PACKAGES_TIDB_TESTS) -coverprofile=coverage.txt -check.p true > gotest.log || { $(FAILPOINT_DISABLE); cat 'gotest.log'; exit 1; }
 	@$(FAILPOINT_DISABLE)
 
 race: failpoint-enable

bindinfo/bind_serial_test.go

@@ -555,7 +555,7 @@ func TestHintsSetID(t *testing.T) {
 
 	utilCleanBindingEnv(tk, dom)
 	err := tk.ExecToErr("create global binding for select * from t using select /*+ non_exist_hint() */ * from t")
-	require.True(t, terror.ErrorEqual(err, parser.ErrWarnOptimizerHintParseError))
+	require.True(t, terror.ErrorEqual(err, parser.ErrParse))
 	tk.MustExec("create global binding for select * from t where a > 10 using select * from t where a > 10")
 	bindData = bindHandle.GetBindRecord(hash, sql, "test")
 	require.NotNil(t, bindData)

bindinfo/cache.go

@@ -105,7 +105,8 @@ func (br *BindRecord) HasUsingBinding() bool {
 
 // FindBinding find bindings in BindRecord.
 func (br *BindRecord) FindBinding(hint string) *Binding {
-	for _, binding := range br.Bindings {
+	for i := range br.Bindings {
+		binding := br.Bindings[i]
 		if binding.ID == hint {
 			return &binding
 		}
@@ -160,7 +161,8 @@ func merge(lBindRecord, rBindRecord *BindRecord) *BindRecord {
 		return lBindRecord
 	}
 	result := lBindRecord.shallowCopy()
-	for _, rbind := range rBindRecord.Bindings {
+	for i := range rBindRecord.Bindings {
+		rbind := rBindRecord.Bindings[i]
 		found := false
 		for j, lbind := range lBindRecord.Bindings {
 			if lbind.isSame(&rbind) {
@@ -184,7 +186,8 @@ func (br *BindRecord) remove(deleted *BindRecord) *BindRecord {
 		return &BindRecord{OriginalSQL: br.OriginalSQL, Db: br.Db}
 	}
 	result := br.shallowCopy()
-	for _, deletedBind := range deleted.Bindings {
+	for j := range deleted.Bindings {
+		deletedBind := deleted.Bindings[j]
 		for i, bind := range result.Bindings {
 			if bind.isSame(&deletedBind) {
 				result.Bindings = append(result.Bindings[:i], result.Bindings[i+1:]...)

bindinfo/capture_serial_test.go

@@ -332,9 +332,8 @@ func TestCapturedBindingCharset(t *testing.T) {
 	require.Len(t, rows, 1)
 	require.Equal(t, "update `test` . `t` set `name` = ? where `name` <= ?", rows[0][0])
 	require.Equal(t, "UPDATE /*+ use_index(@`upd_1` `test`.`t` `idx`)*/ `test`.`t` SET `name`='hello' WHERE `name` <= 'abc'", rows[0][1])
-	// Charset and Collation are empty now, they are not used currently.
-	require.Equal(t, "", rows[0][6])
-	require.Equal(t, "", rows[0][7])
+	require.Equal(t, "utf8mb4", rows[0][6])
+	require.Equal(t, "utf8mb4_bin", rows[0][7])
 }
 
 func TestConcurrentCapture(t *testing.T) {

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

@@ -229,8 +229,8 @@ func checkpointErrorDestroy(ctx context.Context, cfg *config.Config, tls *common
 			for engineID := table.MinEngineID; engineID <= table.MaxEngineID; engineID++ {
 				fmt.Fprintln(os.Stderr, "Closing and cleaning up engine:", table.TableName, engineID)
 				_, eID := backend.MakeUUID(table.TableName, engineID)
-				file := local.File{UUID: eID}
-				err := file.Cleanup(cfg.TikvImporter.SortedKVDir)
+				engine := local.Engine{UUID: eID}
+				err := engine.Cleanup(cfg.TikvImporter.SortedKVDir)
 				if err != nil {
 					fmt.Fprintln(os.Stderr, "* Encountered error while cleanup engine:", err)
 					lastErr = err

br/cmd/tidb-lightning/main.go

@@ -30,7 +30,10 @@ import (
 
 func main() {
 	globalCfg := config.Must(config.LoadGlobalConfig(os.Args[1:], nil))
-	fmt.Fprintf(os.Stdout, "Verbose debug logs will be written to %s\n\n", globalCfg.App.Config.File)
+	logToFile := globalCfg.App.File != "" && globalCfg.App.File != "-"
+	if logToFile {
+		fmt.Fprintf(os.Stdout, "Verbose debug logs will be written to %s\n\n", globalCfg.App.Config.File)
+	}
 
 	app := lightning.New(globalCfg)
 
@@ -95,7 +98,7 @@ func main() {
 	}
 
 	// call Sync() with log to stdout may return error in some case, so just skip it
-	if globalCfg.App.File != "" {
+	if logToFile {
 		syncErr := logger.Sync()
 		if syncErr != nil {
 			fmt.Fprintln(os.Stderr, "sync log failed", syncErr)

br/pkg/backup/schema.go

@@ -88,10 +88,13 @@ func (ss *Schemas) BackupSchemas(
 	metaWriter.StartWriteMetasAsync(ctx, op)
 	for _, s := range ss.schemas {
 		schema := s
+		// Because schema.dbInfo is a pointer that many tables point to.
+		// Remove "add Temporary-prefix into dbName" from closure to prevent concurrent operations.
+		if utils.IsSysDB(schema.dbInfo.Name.L) {
+			schema.dbInfo.Name = utils.TemporaryDBName(schema.dbInfo.Name.O)
+		}
+
 		workerPool.ApplyOnErrorGroup(errg, func() error {
-			if utils.IsSysDB(schema.dbInfo.Name.L) {
-				schema.dbInfo.Name = utils.TemporaryDBName(schema.dbInfo.Name.O)
-			}
 			logger := log.With(
 				zap.String("db", schema.dbInfo.Name.O),
 				zap.String("table", schema.tableInfo.Name.O),

br/pkg/backup/schema_test.go

@@ -4,7 +4,9 @@ package backup_test
 
 import (
 	"context"
+	"fmt"
 	"math"
+	"strings"
 	"sync/atomic"
 
 	"github.com/golang/protobuf/proto"
@@ -16,6 +18,7 @@ import (
 	"github.com/pingcap/tidb/br/pkg/metautil"
 	"github.com/pingcap/tidb/br/pkg/mock"
 	"github.com/pingcap/tidb/br/pkg/storage"
+	"github.com/pingcap/tidb/br/pkg/utils"
 	"github.com/pingcap/tidb/sessionctx/variable"
 	"github.com/pingcap/tidb/util/testkit"
 	"github.com/pingcap/tidb/util/testleak"
@@ -260,3 +263,42 @@ func (s *testBackupSchemaSuite) TestBuildBackupRangeAndSchemaWithBrokenStats(c *
 	c.Assert(schemas2[0].Info, DeepEquals, schemas[0].Info)
 	c.Assert(schemas2[0].DB, DeepEquals, schemas[0].DB)
 }
+
+func (s *testBackupSchemaSuite) TestBackupSchemasForSystemTable(c *C) {
+	tk := testkit.NewTestKit(c, s.mock.Storage)
+	es2 := s.GetRandomStorage(c)
+
+	systemTablesCount := 32
+	tablePrefix := "systable"
+	tk.MustExec("use mysql")
+	for i := 1; i <= systemTablesCount; i++ {
+		query := fmt.Sprintf("create table %s%d (a char(1));", tablePrefix, i)
+		tk.MustExec(query)
+	}
+
+	f, err := filter.Parse([]string{"mysql.systable*"})
+	c.Assert(err, IsNil)
+	_, backupSchemas, err := backup.BuildBackupRangeAndSchema(s.mock.Storage, f, math.MaxUint64)
+	c.Assert(err, IsNil)
+	c.Assert(backupSchemas.Len(), Equals, systemTablesCount)
+
+	ctx := context.Background()
+	cipher := backuppb.CipherInfo{
+		CipherType: encryptionpb.EncryptionMethod_PLAINTEXT,
+	}
+	updateCh := new(simpleProgress)
+
+	metaWriter2 := metautil.NewMetaWriter(es2, metautil.MetaFileSize, false, &cipher)
+	err = backupSchemas.BackupSchemas(ctx, metaWriter2, s.mock.Storage, nil,
+		math.MaxUint64, 1, variable.DefChecksumTableConcurrency, true, updateCh)
+	c.Assert(err, IsNil)
+	err = metaWriter2.FlushBackupMeta(ctx)
+	c.Assert(err, IsNil)
+
+	schemas2 := s.GetSchemasFromMeta(c, es2)
+	c.Assert(schemas2, HasLen, systemTablesCount)
+	for _, schema := range schemas2 {
+		c.Assert(schema.DB.Name, Equals, utils.TemporaryDBName("mysql"))
+		c.Assert(strings.HasPrefix(schema.Info.Name.O, tablePrefix), Equals, true)
+	}
+}

br/pkg/lightning/backend/local/duplicate.go

@@ -494,6 +494,7 @@ func (manager *DuplicateManager) CollectDuplicateRowsFromLocalIndex(
 				defer iter.Close()
 
 				for iter.First(); iter.Valid(); iter.Next() {
+					hasDataConflict = true
 					rawKey, _, _, err := manager.keyAdapter.Decode(nil, iter.Key())
 					if err != nil {
 						return err
@@ -525,7 +526,6 @@ func (manager *DuplicateManager) CollectDuplicateRowsFromLocalIndex(
 					return err
 				}
 				dataConflictInfos = dataConflictInfos[:0]
-				hasDataConflict = true
 				return nil
 			}(); err != nil {
 				return false, errors.Trace(err)
@@ -569,6 +569,7 @@ func (manager *DuplicateManager) CollectDuplicateRowsFromLocalIndex(
 				defer iter.Close()
 
 				for iter.First(); iter.Valid(); iter.Next() {
+					hasDataConflict = true
 					rawKey, _, _, err := manager.keyAdapter.Decode(nil, iter.Key())
 					if err != nil {
 						indexLogger.Error(