From cde41970e2c9b97ffae58e16543b95b85c171b0f Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Sat, 14 Mar 2020 16:00:04 +0800 Subject: [PATCH 01/12] refline logs --- cmd/cmd.go | 4 ++-- pkg/summary/collector.go | 10 +++++++++- pkg/task/backup.go | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 5b2801894..d08caf446 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -49,8 +49,8 @@ func AddFlags(cmd *cobra.Command) { cmd.PersistentFlags().StringP(FlagLogLevel, "L", "info", "Set the log level") - cmd.PersistentFlags().String(FlagLogFile, "", - "Set the log file path. If not set, logs will output to stdout") + cmd.PersistentFlags().String(FlagLogFile, "/tmp/br.log", + "Set the log file path. If not set, logs will output to /tmp/br.log") cmd.PersistentFlags().String(FlagStatusAddr, "", "Set the HTTP listening address for the status report service. Set to empty string to disable") task.DefineCommonFlags(cmd.PersistentFlags()) diff --git a/pkg/summary/collector.go b/pkg/summary/collector.go index 42488cb82..e8c40ea01 100644 --- a/pkg/summary/collector.go +++ b/pkg/summary/collector.go @@ -40,7 +40,15 @@ type LogCollector interface { type logFunc func(msg string, fields ...zap.Field) -var collector = newLogCollector(log.Info) +var collector = func() LogCollector { + conf := new(log.Config) + // Always output summary to stdout. + logger, _, err := log.InitLogger(conf) + if err != nil { + logger = log.L() + } + return newLogCollector(logger.Info) +}() type logCollector struct { mu sync.Mutex diff --git a/pkg/task/backup.go b/pkg/task/backup.go index ab22c9039..3000688a6 100644 --- a/pkg/task/backup.go +++ b/pkg/task/backup.go @@ -213,6 +213,7 @@ func RunBackup(c context.Context, g glue.Glue, cmdName string, cfg *BackupConfig err = client.SaveBackupMeta(ctx, ddlJobs) if err != nil { + log.Error("save backup meta failed", zap.Reflect("meta", ddlJobs)) return err } return nil From b69990df1a910d0fb6ff827930b151ee398f9c2e Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Sat, 14 Mar 2020 16:13:43 +0800 Subject: [PATCH 02/12] add log on restore --- pkg/task/common.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/task/common.go b/pkg/task/common.go index 80f5eb258..30c2a16c4 100644 --- a/pkg/task/common.go +++ b/pkg/task/common.go @@ -12,12 +12,14 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/backup" + "github.com/pingcap/log" pd "github.com/pingcap/pd/v4/client" "github.com/pingcap/tidb-tools/pkg/filter" "github.com/pingcap/tidb/store/tikv" "github.com/spf13/cobra" "github.com/spf13/pflag" "go.etcd.io/etcd/pkg/transport" + "go.uber.org/zap" "github.com/pingcap/br/pkg/conn" "github.com/pingcap/br/pkg/glue" @@ -268,6 +270,7 @@ func ReadBackupMeta( } backupMeta := &backup.BackupMeta{} if err = proto.Unmarshal(metaData, backupMeta); err != nil { + log.Error("parse backupmeta failed", zap.Error(err), zap.Binary(metaData)) return nil, nil, nil, errors.Annotate(err, "parse backupmeta failed") } return u, s, backupMeta, nil From 4539ca4907cf2e7c90dc785055c7c67d5d85d2af Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Sat, 14 Mar 2020 16:16:16 +0800 Subject: [PATCH 03/12] fix --- pkg/task/common.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/task/common.go b/pkg/task/common.go index 30c2a16c4..309be6349 100644 --- a/pkg/task/common.go +++ b/pkg/task/common.go @@ -270,7 +270,7 @@ func ReadBackupMeta( } backupMeta := &backup.BackupMeta{} if err = proto.Unmarshal(metaData, backupMeta); err != nil { - log.Error("parse backupmeta failed", zap.Error(err), zap.Binary(metaData)) + log.Error("parse backupmeta failed", zap.Error(err), zap.Binary("meta", metaData)) return nil, nil, nil, errors.Annotate(err, "parse backupmeta failed") } return u, s, backupMeta, nil From 83233cdca80272413a94f21cecc7d032e07e0bff Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Sat, 14 Mar 2020 16:20:23 +0800 Subject: [PATCH 04/12] always log backup meta --- pkg/task/restore.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/pkg/task/restore.go b/pkg/task/restore.go index a02e49cf1..cf9507266 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -71,7 +71,7 @@ func (cfg *RestoreConfig) ParseFromFlags(flags *pflag.FlagSet) error { } // RunRestore starts a restore task inside the current goroutine. -func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConfig) error { +func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConfig) (err error) { ctx, cancel := context.WithCancel(c) defer cancel() @@ -103,6 +103,13 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err != nil { return err } + + defer func() { + if err != nil { + log.Error("restore failed", zap.Error(err), zap.Reflect("meta", backupMeta)) + } + }() + if err = client.InitBackupMeta(backupMeta, u); err != nil { return err } From b60ecb4dfd21276e6623ab26134cbab99afdf3c8 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Sun, 15 Mar 2020 14:53:49 +0800 Subject: [PATCH 05/12] timestamp log file --- cmd/cmd.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index d08caf446..5156beef0 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -4,10 +4,13 @@ package cmd import ( "context" + "fmt" "net/http" "net/http/pprof" + "os" "sync" "sync/atomic" + "time" "github.com/pingcap/log" "github.com/pingcap/tidb/util/logutil" @@ -41,6 +44,10 @@ const ( flagVersionShort = "V" ) +func timestampLogFileName() string { + return fmt.Sprintf("%s/br-%s", os.TempDir(), time.Now().Format(time.RFC3339)) +} + // AddFlags adds flags to the given cmd. func AddFlags(cmd *cobra.Command) { cmd.Version = utils.BRInfo() @@ -49,8 +56,7 @@ func AddFlags(cmd *cobra.Command) { cmd.PersistentFlags().StringP(FlagLogLevel, "L", "info", "Set the log level") - cmd.PersistentFlags().String(FlagLogFile, "/tmp/br.log", - "Set the log file path. If not set, logs will output to /tmp/br.log") + cmd.PersistentFlags().String(FlagLogFile, timestampLogFileName(), "Set the log file path. If not set, logs will output to temp file") cmd.PersistentFlags().String(FlagStatusAddr, "", "Set the HTTP listening address for the status report service. Set to empty string to disable") task.DefineCommonFlags(cmd.PersistentFlags()) @@ -75,6 +81,8 @@ func Init(cmd *cobra.Command) (err error) { } if len(conf.File.Filename) != 0 { atomic.StoreUint64(&hasLogFile, 1) + } else { + fmt.Printf("log file: %s\n", conf.File.Filename) } lg, p, e := log.InitLogger(conf) if e != nil { From 2c430a8f508a94fd97d7e396b569faf0fd4eb662 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BA=84=E5=A4=A9=E7=BF=BC?= Date: Tue, 17 Mar 2020 21:55:19 +0800 Subject: [PATCH 06/12] Update cmd/cmd.go Co-Authored-By: kennytm --- cmd/cmd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 5156beef0..b6aec4941 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -45,7 +45,7 @@ const ( ) func timestampLogFileName() string { - return fmt.Sprintf("%s/br-%s", os.TempDir(), time.Now().Format(time.RFC3339)) + return filepath.Join(os.TempDir(), "br-" + time.Now().Format(time.RFC3339)) } // AddFlags adds flags to the given cmd. From cbe57e1078fb7ecc353661bea1ea6eb75556ee58 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Tue, 17 Mar 2020 21:54:35 +0800 Subject: [PATCH 07/12] duplicate summary output --- cmd/cmd.go | 2 ++ pkg/summary/collector.go | 25 +++++++++++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index b6aec4941..2fc472706 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -19,6 +19,7 @@ import ( "go.uber.org/zap" "github.com/pingcap/br/pkg/gluetidb" + "github.com/pingcap/br/pkg/summary" "github.com/pingcap/br/pkg/task" "github.com/pingcap/br/pkg/utils" ) @@ -84,6 +85,7 @@ func Init(cmd *cobra.Command) (err error) { } else { fmt.Printf("log file: %s\n", conf.File.Filename) } + summary.InitCollector(hasLogFile) lg, p, e := log.InitLogger(conf) if e != nil { err = e diff --git a/pkg/summary/collector.go b/pkg/summary/collector.go index e8c40ea01..1e6f91dc5 100644 --- a/pkg/summary/collector.go +++ b/pkg/summary/collector.go @@ -40,15 +40,24 @@ type LogCollector interface { type logFunc func(msg string, fields ...zap.Field) -var collector = func() LogCollector { - conf := new(log.Config) - // Always output summary to stdout. - logger, _, err := log.InitLogger(conf) - if err != nil { - logger = log.L() +var collector LogCollector = newLogCollector(log.Info) + +// InitCollector initilize global collector instance. +func InitCollector(hasLogFile bool) { + logF := log.L().Info + if hasLogFile { + conf := new(log.Config) + // Always duplicate summary to stdout. + logger, _, err := log.InitLogger(conf) + if err == nil { + logF = func(msg string, fields ...zap.Field) { + logger.Info(msg, fields...) + log.Info(msg, fields...) + } + } } - return newLogCollector(logger.Info) -}() + collector = newLogCollector(logF) +} type logCollector struct { mu sync.Mutex From 52c652cf826e9b939c79c12da753c77c732481df Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Tue, 17 Mar 2020 21:58:38 +0800 Subject: [PATCH 08/12] fix --- cmd/cmd.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 2fc472706..8d80ab80e 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -85,7 +85,7 @@ func Init(cmd *cobra.Command) (err error) { } else { fmt.Printf("log file: %s\n", conf.File.Filename) } - summary.InitCollector(hasLogFile) + summary.InitCollector(bool(hasLogFile)) lg, p, e := log.InitLogger(conf) if e != nil { err = e From 4ed1a35ddd965d8231ef4a985956b8213a7517c5 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Tue, 17 Mar 2020 22:01:14 +0800 Subject: [PATCH 09/12] remove meta and fix --- cmd/cmd.go | 5 +++-- pkg/task/backup.go | 1 - pkg/task/common.go | 3 --- pkg/task/restore.go | 6 ------ 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 8d80ab80e..d40d35766 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/pprof" "os" + "path/filepath" "sync" "sync/atomic" "time" @@ -46,7 +47,7 @@ const ( ) func timestampLogFileName() string { - return filepath.Join(os.TempDir(), "br-" + time.Now().Format(time.RFC3339)) + return filepath.Join(os.TempDir(), "br-"+time.Now().Format(time.RFC3339)) } // AddFlags adds flags to the given cmd. @@ -82,10 +83,10 @@ func Init(cmd *cobra.Command) (err error) { } if len(conf.File.Filename) != 0 { atomic.StoreUint64(&hasLogFile, 1) + summary.InitCollector(true) } else { fmt.Printf("log file: %s\n", conf.File.Filename) } - summary.InitCollector(bool(hasLogFile)) lg, p, e := log.InitLogger(conf) if e != nil { err = e diff --git a/pkg/task/backup.go b/pkg/task/backup.go index 529fd071e..8d9613047 100644 --- a/pkg/task/backup.go +++ b/pkg/task/backup.go @@ -214,7 +214,6 @@ func RunBackup(c context.Context, g glue.Glue, cmdName string, cfg *BackupConfig err = client.SaveBackupMeta(ctx, ddlJobs) if err != nil { - log.Error("save backup meta failed", zap.Reflect("meta", ddlJobs)) return err } return nil diff --git a/pkg/task/common.go b/pkg/task/common.go index 3785c2928..859c4206d 100644 --- a/pkg/task/common.go +++ b/pkg/task/common.go @@ -12,14 +12,12 @@ import ( "github.com/gogo/protobuf/proto" "github.com/pingcap/errors" "github.com/pingcap/kvproto/pkg/backup" - "github.com/pingcap/log" pd "github.com/pingcap/pd/v4/client" "github.com/pingcap/tidb-tools/pkg/filter" "github.com/pingcap/tidb/store/tikv" "github.com/spf13/cobra" "github.com/spf13/pflag" "go.etcd.io/etcd/pkg/transport" - "go.uber.org/zap" "github.com/pingcap/br/pkg/conn" "github.com/pingcap/br/pkg/glue" @@ -276,7 +274,6 @@ func ReadBackupMeta( } backupMeta := &backup.BackupMeta{} if err = proto.Unmarshal(metaData, backupMeta); err != nil { - log.Error("parse backupmeta failed", zap.Error(err), zap.Binary("meta", metaData)) return nil, nil, nil, errors.Annotate(err, "parse backupmeta failed") } return u, s, backupMeta, nil diff --git a/pkg/task/restore.go b/pkg/task/restore.go index 5ca00f693..8b5e26199 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -104,12 +104,6 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf return err } - defer func() { - if err != nil { - log.Error("restore failed", zap.Error(err), zap.Reflect("meta", backupMeta)) - } - }() - if err = client.InitBackupMeta(backupMeta, u); err != nil { return err } From 908d838e7099ef4f159e35bcec900bf2bc1a50c3 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Tue, 17 Mar 2020 22:03:14 +0800 Subject: [PATCH 10/12] useless blank --- pkg/task/restore.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/task/restore.go b/pkg/task/restore.go index 8b5e26199..d45d1a4af 100644 --- a/pkg/task/restore.go +++ b/pkg/task/restore.go @@ -103,7 +103,6 @@ func RunRestore(c context.Context, g glue.Glue, cmdName string, cfg *RestoreConf if err != nil { return err } - if err = client.InitBackupMeta(backupMeta, u); err != nil { return err } From ebf99000383ee552c4216928ed603f49e7b572f0 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Wed, 18 Mar 2020 21:02:28 +0800 Subject: [PATCH 11/12] use cmd --- cmd/cmd.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index d40d35766..0f06d5599 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -4,7 +4,6 @@ package cmd import ( "context" - "fmt" "net/http" "net/http/pprof" "os" @@ -85,7 +84,7 @@ func Init(cmd *cobra.Command) (err error) { atomic.StoreUint64(&hasLogFile, 1) summary.InitCollector(true) } else { - fmt.Printf("log file: %s\n", conf.File.Filename) + cmd.Printf("log file: %s\n", conf.File.Filename) } lg, p, e := log.InitLogger(conf) if e != nil { From 521a39447210b44f453f7dde1908231e0a8d69e3 Mon Sep 17 00:00:00 2001 From: TennyZhuang Date: Thu, 19 Mar 2020 16:04:26 +0800 Subject: [PATCH 12/12] make ci happy --- cmd/cmd.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/cmd.go b/cmd/cmd.go index 0f06d5599..3e6abff67 100644 --- a/cmd/cmd.go +++ b/cmd/cmd.go @@ -57,7 +57,8 @@ func AddFlags(cmd *cobra.Command) { cmd.PersistentFlags().StringP(FlagLogLevel, "L", "info", "Set the log level") - cmd.PersistentFlags().String(FlagLogFile, timestampLogFileName(), "Set the log file path. If not set, logs will output to temp file") + cmd.PersistentFlags().String(FlagLogFile, timestampLogFileName(), + "Set the log file path. If not set, logs will output to temp file") cmd.PersistentFlags().String(FlagStatusAddr, "", "Set the HTTP listening address for the status report service. Set to empty string to disable") task.DefineCommonFlags(cmd.PersistentFlags())