From a9aeb7d52188d00dbb1bf1ca12f493fa7b41b9ff Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Wed, 12 Oct 2022 17:26:25 -0700 Subject: [PATCH 1/3] remove excessive logging Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/builtinbackupengine.go | 11 +++++++++++ go/vt/mysqlctl/xtrabackupengine.go | 12 +++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 09323c9387b..31e250c6302 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -606,6 +606,17 @@ func (be *BuiltinBackupEngine) ExecuteRestore(ctx context.Context, params Restor // restoreFiles will copy all the files from the BackupStorage to the // right place. func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle, bm builtinBackupManifest) error { + // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file + // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. + if bm.CompressionEngine == PargzipCompressor { + originalEngineValue := bm.CompressionEngine + params.Logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + bm.CompressionEngine = PgzipCompressor + defer func() { + bm.CompressionEngine = originalEngineValue + }() + } + fes := bm.FileEntries sema := sync2.NewSemaphore(params.Concurrency, 0) rec := concurrency.AllErrorRecorder{} diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index d0d131e3cee..8a1b551b8e7 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -451,7 +451,6 @@ func (be *XtrabackupEngine) ExecuteRestore(ctx context.Context, params RestorePa func (be *XtrabackupEngine) restoreFromBackup(ctx context.Context, cnf *Mycnf, bh backupstorage.BackupHandle, bm xtraBackupManifest, logger logutil.Logger) error { // first download the file into a tmp dir // and extract all the files - tempDir := fmt.Sprintf("%v/%v", cnf.TmpDir, time.Now().UTC().Format("xtrabackup-2006-01-02.150405")) // create tempDir if err := os.MkdirAll(tempDir, os.ModePerm); err != nil { @@ -465,6 +464,17 @@ func (be *XtrabackupEngine) restoreFromBackup(ctx context.Context, cnf *Mycnf, b } }(tempDir, logger) + // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file + // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. + if bm.CompressionEngine == PargzipCompressor { + originalEngineValue := bm.CompressionEngine + logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + bm.CompressionEngine = PgzipCompressor + defer func() { + bm.CompressionEngine = originalEngineValue + }() + } + if err := be.extractFiles(ctx, logger, bh, bm, tempDir); err != nil { logger.Errorf("error extracting backup files: %v", err) return err From 06891b246cc7d35b4e2597727d5daf419cc5b78a Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Fri, 14 Oct 2022 12:16:31 -0700 Subject: [PATCH 2/3] code feeback Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/builtinbackupengine.go | 5 ++--- go/vt/mysqlctl/compression.go | 2 +- go/vt/mysqlctl/xtrabackupengine.go | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 31e250c6302..e25d80c227a 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -609,11 +609,10 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. if bm.CompressionEngine == PargzipCompressor { - originalEngineValue := bm.CompressionEngine - params.Logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + params.Logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) bm.CompressionEngine = PgzipCompressor defer func() { - bm.CompressionEngine = originalEngineValue + bm.CompressionEngine = PargzipCompressor }() } diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index 40c4dc344a3..6e069bd1fd1 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -193,7 +193,7 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade // This returns a reader that will decompress the underlying provided reader and will use the specified supported engine. func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { if engine == PargzipCompressor { - logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) engine = PgzipCompressor } diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 8a1b551b8e7..23ad7a48aef 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -467,11 +467,10 @@ func (be *XtrabackupEngine) restoreFromBackup(ctx context.Context, cnf *Mycnf, b // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. if bm.CompressionEngine == PargzipCompressor { - originalEngineValue := bm.CompressionEngine - logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) bm.CompressionEngine = PgzipCompressor defer func() { - bm.CompressionEngine = originalEngineValue + bm.CompressionEngine = PargzipCompressor }() } From 2d5ac1e61e2b3a17e59df822fffad4a823438d1b Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Fri, 14 Oct 2022 15:41:05 -0700 Subject: [PATCH 3/3] fixing typo Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/builtinbackupengine.go | 2 +- go/vt/mysqlctl/compression.go | 2 +- go/vt/mysqlctl/xtrabackupengine.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index e25d80c227a..525b8578d44 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -609,7 +609,7 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. if bm.CompressionEngine == PargzipCompressor { - params.Logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) + params.Logger.Warningf(`engine "pargzip" doesn't support decompression, using "pgzip" instead`) bm.CompressionEngine = PgzipCompressor defer func() { bm.CompressionEngine = PargzipCompressor diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index 6e069bd1fd1..3d6b017a70b 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -193,7 +193,7 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade // This returns a reader that will decompress the underlying provided reader and will use the specified supported engine. func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { if engine == PargzipCompressor { - logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) + logger.Warningf(`engine "pargzip" doesn't support decompression, using "pgzip" instead`) engine = PgzipCompressor } diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 23ad7a48aef..112bdf4179d 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -467,7 +467,7 @@ func (be *XtrabackupEngine) restoreFromBackup(ctx context.Context, cnf *Mycnf, b // For optimization, we are replacing pargzip with pgzip, so newBuiltinDecompressor doesn't have to compare and print warning for every file // since newBuiltinDecompressor is helper method and does not hold any state, it was hard to do it in that method itself. if bm.CompressionEngine == PargzipCompressor { - logger.Warningf(`engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead`) + logger.Warningf(`engine "pargzip" doesn't support decompression, using "pgzip" instead`) bm.CompressionEngine = PgzipCompressor defer func() { bm.CompressionEngine = PargzipCompressor