From e9fc0ade8e3ceac276336a825336e439652c21f4 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, 8 Nov 2023 21:12:42 +0800 Subject: [PATCH 1/3] This is an automated cherry-pick of #48025 Signed-off-by: ti-chi-bot --- br/pkg/task/BUILD.bazel | 10 ++++- br/pkg/task/backup.go | 20 +++++++++ br/pkg/task/common.go | 10 +++++ br/pkg/task/common_test.go | 84 ++++++++++++++++++++++++++++++++++++++ br/pkg/task/restore.go | 17 ++++++++ executor/brie.go | 30 +++++++------- 6 files changed, 154 insertions(+), 17 deletions(-) diff --git a/br/pkg/task/BUILD.bazel b/br/pkg/task/BUILD.bazel index cca60076b1071..e33d9a1a20e12 100644 --- a/br/pkg/task/BUILD.bazel +++ b/br/pkg/task/BUILD.bazel @@ -98,7 +98,7 @@ go_test( ], embed = [":task"], flaky = True, - shard_count = 18, + shard_count = 21, deps = [ "//br/pkg/conn", "//br/pkg/errors", @@ -107,10 +107,18 @@ go_test( "//br/pkg/storage", "//br/pkg/stream", "//br/pkg/utils", +<<<<<<< HEAD "//config", "//parser/model", "//statistics/handle", "//tablecodec", +======= + "//pkg/config", + "//pkg/parser/model", + "//pkg/statistics/handle/util", + "//pkg/tablecodec", + "//pkg/util/table-filter", +>>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) "@com_github_golang_protobuf//proto", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/brpb", diff --git a/br/pkg/task/backup.go b/br/pkg/task/backup.go index 36ca6b5925091..86cc602db092c 100644 --- a/br/pkg/task/backup.go +++ b/br/pkg/task/backup.go @@ -38,6 +38,11 @@ import ( "github.com/pingcap/tidb/util/mathutil" "github.com/spf13/pflag" "github.com/tikv/client-go/v2/oracle" +<<<<<<< HEAD +======= + kvutil "github.com/tikv/client-go/v2/util" + "go.uber.org/multierr" +>>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) "go.uber.org/zap" ) @@ -773,6 +778,21 @@ func ParseTSString(ts string, tzCheck bool) (uint64, error) { return oracle.GoTimeToTS(t1), nil } +func DefaultBackupConfig() BackupConfig { + fs := pflag.NewFlagSet("dummy", pflag.ContinueOnError) + DefineCommonFlags(fs) + DefineBackupFlags(fs) + cfg := BackupConfig{} + err := multierr.Combine( + cfg.ParseFromFlags(fs), + cfg.Config.ParseFromFlags(fs), + ) + if err != nil { + log.Panic("infallible operation failed.", zap.Error(err)) + } + return cfg +} + func parseCompressionType(s string) (backuppb.CompressionType, error) { var ct backuppb.CompressionType switch s { diff --git a/br/pkg/task/common.go b/br/pkg/task/common.go index 2f1fca8cdae89..6c2a19af4fae4 100644 --- a/br/pkg/task/common.go +++ b/br/pkg/task/common.go @@ -330,6 +330,16 @@ func HiddenFlagsForStream(flags *pflag.FlagSet) { storage.HiddenFlagsForStream(flags) } +func DefaultConfig() Config { + fs := pflag.NewFlagSet("dummy", pflag.ContinueOnError) + DefineCommonFlags(fs) + cfg := Config{} + if err := cfg.ParseFromFlags(fs); err != nil { + log.Panic("infallible operation failed.", zap.Error(err)) + } + return cfg +} + // DefineDatabaseFlags defines the required --db flag for `db` subcommand. func DefineDatabaseFlags(command *cobra.Command) { command.Flags().String(flagDatabase, "", "database name") diff --git a/br/pkg/task/common_test.go b/br/pkg/task/common_test.go index b124f6977b9fa..be16f87a3aae4 100644 --- a/br/pkg/task/common_test.go +++ b/br/pkg/task/common_test.go @@ -9,7 +9,14 @@ import ( backup "github.com/pingcap/kvproto/pkg/brpb" "github.com/pingcap/kvproto/pkg/encryptionpb" +<<<<<<< HEAD "github.com/pingcap/tidb/config" +======= + "github.com/pingcap/tidb/br/pkg/storage" + "github.com/pingcap/tidb/br/pkg/utils" + "github.com/pingcap/tidb/pkg/config" + filter "github.com/pingcap/tidb/pkg/util/table-filter" +>>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) "github.com/spf13/pflag" "github.com/stretchr/testify/require" ) @@ -157,3 +164,80 @@ func TestCheckCipherKey(t *testing.T) { } } } + +func must[T any](t T, err error) T { + if err != nil { + panic(err) + } + return t +} + +func expectedDefaultConfig() Config { + return Config{ + BackendOptions: storage.BackendOptions{S3: storage.S3BackendOptions{ForcePathStyle: true}}, + PD: []string{"127.0.0.1:2379"}, + ChecksumConcurrency: 4, + Checksum: true, + SendCreds: true, + CheckRequirements: true, + FilterStr: []string(nil), + TableFilter: filter.CaseInsensitive(must(filter.Parse([]string{"*.*"}))), + Schemas: map[string]struct{}{}, + Tables: map[string]struct{}{}, + SwitchModeInterval: 300000000000, + GRPCKeepaliveTime: 10000000000, + GRPCKeepaliveTimeout: 3000000000, + CipherInfo: backup.CipherInfo{CipherType: 1}, + MetadataDownloadBatchSize: 0x80, + } +} + +func expectedDefaultBackupConfig() BackupConfig { + return BackupConfig{ + Config: expectedDefaultConfig(), + GCTTL: utils.DefaultBRGCSafePointTTL, + CompressionConfig: CompressionConfig{ + CompressionType: backup.CompressionType_ZSTD, + }, + IgnoreStats: true, + UseBackupMetaV2: true, + UseCheckpoint: true, + } +} + +func expectedDefaultRestoreConfig() RestoreConfig { + defaultConfig := expectedDefaultConfig() + defaultConfig.Concurrency = defaultRestoreConcurrency + return RestoreConfig{ + Config: defaultConfig, + RestoreCommonConfig: RestoreCommonConfig{Online: false, + MergeSmallRegionSizeBytes: 0x6000000, + MergeSmallRegionKeyCount: 0xea600, + WithSysTable: false, + ResetSysUsers: []string{"cloud_admin", "root"}}, + NoSchema: false, + PDConcurrency: 0x1, + BatchFlushInterval: 16000000000, + DdlBatchSize: 0x80, + WithPlacementPolicy: "STRICT", + UseCheckpoint: true, + } +} + +func TestDefault(t *testing.T) { + def := DefaultConfig() + defaultConfig := expectedDefaultConfig() + require.Equal(t, defaultConfig, def) +} + +func TestDefaultBackup(t *testing.T) { + def := DefaultBackupConfig() + defaultConfig := expectedDefaultBackupConfig() + require.Equal(t, defaultConfig, def) +} + +func TestDefaultRestore(t *testing.T) { + def := DefaultRestoreConfig() + defaultConfig := expectedDefaultRestoreConfig() + require.Equal(t, defaultConfig, def) +} diff --git a/br/pkg/task/restore.go b/br/pkg/task/restore.go index d96f129036114..998e435d4236e 100644 --- a/br/pkg/task/restore.go +++ b/br/pkg/task/restore.go @@ -562,6 +562,23 @@ func removeCheckpointDataForLogRestore(ctx context.Context, storageName string, return errors.Trace(checkpoint.RemoveCheckpointDataForLogRestore(ctx, s, taskName, clusterID)) } +func DefaultRestoreConfig() RestoreConfig { + fs := pflag.NewFlagSet("dummy", pflag.ContinueOnError) + DefineCommonFlags(fs) + DefineRestoreFlags(fs) + cfg := RestoreConfig{} + err := multierr.Combine( + cfg.ParseFromFlags(fs), + cfg.RestoreCommonConfig.ParseFromFlags(fs), + cfg.Config.ParseFromFlags(fs), + ) + if err != nil { + log.Panic("infallible failed.", zap.Error(err)) + } + + return cfg +} + // RunRestore starts a restore task inside the current goroutine. func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConfig) error { etcdCLI, err := dialEtcdWithCfg(c, cfg.Config) diff --git a/executor/brie.go b/executor/brie.go index 6eef77750967d..50c463e8c03c0 100644 --- a/executor/brie.go +++ b/executor/brie.go @@ -228,21 +228,15 @@ func (b *executorBuilder) buildBRIE(s *ast.BRIEStmt, schema *expression.Schema) } tidbCfg := config.GetGlobalConfig() - cfg := task.Config{ - TLS: task.TLSConfig{ - CA: tidbCfg.Security.ClusterSSLCA, - Cert: tidbCfg.Security.ClusterSSLCert, - Key: tidbCfg.Security.ClusterSSLKey, - }, - PD: strings.Split(tidbCfg.Path, ","), - Concurrency: 4, - Checksum: true, - SendCreds: true, - LogProgress: true, - CipherInfo: backuppb.CipherInfo{ - CipherType: encryptionpb.EncryptionMethod_PLAINTEXT, - }, + tlsCfg := task.TLSConfig{ + CA: tidbCfg.Security.ClusterSSLCA, + Cert: tidbCfg.Security.ClusterSSLCert, + Key: tidbCfg.Security.ClusterSSLKey, } + pds := strings.Split(tidbCfg.Path, ",") + cfg := task.DefaultConfig() + cfg.PD = pds + cfg.TLS = tlsCfg storageURL, err := storage.ParseRawURL(s.Storage) if err != nil { @@ -310,7 +304,9 @@ func (b *executorBuilder) buildBRIE(s *ast.BRIEStmt, schema *expression.Schema) switch s.Kind { case ast.BRIEKindBackup: - e.backupCfg = &task.BackupConfig{Config: cfg} + bcfg := task.DefaultBackupConfig() + bcfg.Config = cfg + e.backupCfg = &bcfg for _, opt := range s.Options { switch opt.Tp { @@ -338,7 +334,9 @@ func (b *executorBuilder) buildBRIE(s *ast.BRIEStmt, schema *expression.Schema) } case ast.BRIEKindRestore: - e.restoreCfg = &task.RestoreConfig{Config: cfg} + rcfg := task.DefaultRestoreConfig() + rcfg.Config = cfg + e.restoreCfg = &rcfg for _, opt := range s.Options { switch opt.Tp { case ast.BRIEOptionOnline: From dcff3314e87225c2457dd5b75a292013331731e5 Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 5 Dec 2023 15:27:02 +0800 Subject: [PATCH 2/3] resolve conflicts && make bazel_prepare Signed-off-by: hillium --- br/pkg/task/BUILD.bazel | 9 +-------- br/pkg/task/backup.go | 4 ---- br/pkg/task/common_test.go | 9 +++------ 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/br/pkg/task/BUILD.bazel b/br/pkg/task/BUILD.bazel index e33d9a1a20e12..e96bfc1ebb4ce 100644 --- a/br/pkg/task/BUILD.bazel +++ b/br/pkg/task/BUILD.bazel @@ -107,18 +107,11 @@ go_test( "//br/pkg/storage", "//br/pkg/stream", "//br/pkg/utils", -<<<<<<< HEAD "//config", "//parser/model", "//statistics/handle", "//tablecodec", -======= - "//pkg/config", - "//pkg/parser/model", - "//pkg/statistics/handle/util", - "//pkg/tablecodec", - "//pkg/util/table-filter", ->>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) + "//util/table-filter", "@com_github_golang_protobuf//proto", "@com_github_pingcap_errors//:errors", "@com_github_pingcap_kvproto//pkg/brpb", diff --git a/br/pkg/task/backup.go b/br/pkg/task/backup.go index 86cc602db092c..5ff29edf64124 100644 --- a/br/pkg/task/backup.go +++ b/br/pkg/task/backup.go @@ -38,11 +38,7 @@ import ( "github.com/pingcap/tidb/util/mathutil" "github.com/spf13/pflag" "github.com/tikv/client-go/v2/oracle" -<<<<<<< HEAD -======= - kvutil "github.com/tikv/client-go/v2/util" "go.uber.org/multierr" ->>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) "go.uber.org/zap" ) diff --git a/br/pkg/task/common_test.go b/br/pkg/task/common_test.go index be16f87a3aae4..5398cb268e7d1 100644 --- a/br/pkg/task/common_test.go +++ b/br/pkg/task/common_test.go @@ -9,14 +9,11 @@ import ( backup "github.com/pingcap/kvproto/pkg/brpb" "github.com/pingcap/kvproto/pkg/encryptionpb" -<<<<<<< HEAD - "github.com/pingcap/tidb/config" -======= "github.com/pingcap/tidb/br/pkg/storage" "github.com/pingcap/tidb/br/pkg/utils" - "github.com/pingcap/tidb/pkg/config" - filter "github.com/pingcap/tidb/pkg/util/table-filter" ->>>>>>> 632cd843b0e (executor/brie: use the default value from flags (#48025)) + "github.com/pingcap/tidb/config" + filter "github.com/pingcap/tidb/util/table-filter" + "github.com/spf13/pflag" "github.com/stretchr/testify/require" ) From 1c050227e1dca52f304f4cb9a488cf8ad998279a Mon Sep 17 00:00:00 2001 From: hillium Date: Tue, 5 Dec 2023 15:38:49 +0800 Subject: [PATCH 3/3] make clippy happy Signed-off-by: hillium --- br/pkg/task/common_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/br/pkg/task/common_test.go b/br/pkg/task/common_test.go index 5398cb268e7d1..858644dc22807 100644 --- a/br/pkg/task/common_test.go +++ b/br/pkg/task/common_test.go @@ -13,7 +13,6 @@ import ( "github.com/pingcap/tidb/br/pkg/utils" "github.com/pingcap/tidb/config" filter "github.com/pingcap/tidb/util/table-filter" - "github.com/spf13/pflag" "github.com/stretchr/testify/require" )