From 69db835ab05da9f5f9c3b4f378b827fb4c78f933 Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Tue, 27 Apr 2021 06:37:56 -0700 Subject: [PATCH 01/21] change to support an external decompressor Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/xtrabackupengine.go | 76 ++++++++++++++++++++++++- go/vt/mysqlctl/xtrabackupengine_test.go | 39 +++++++++++++ 2 files changed, 112 insertions(+), 3 deletions(-) diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 18ced1b375b..45bf6d85e01 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -20,6 +20,7 @@ import ( "bufio" "context" "encoding/json" + "errors" "flag" "fmt" "io" @@ -63,6 +64,9 @@ var ( // striping mode xtrabackupStripes = flag.Uint("xtrabackup_stripes", 0, "If greater than 0, use data striping across this many destination files to parallelize data transfer and decompression") xtrabackupStripeBlockSize = flag.Uint("xtrabackup_stripe_block_size", 102400, "Size in bytes of each block that gets sent to a given stripe before rotating to the next stripe") + // use and external command to decompress the backups + decompressMethod = flag.String("decompress_method", "builtin", "what decompressor to use [builtin|external]") + externalDecompressor = flag.String("external_decompressor", "", "command with arguments to which decompressor to run") ) const ( @@ -521,17 +525,74 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log } }() + var ( + decompressorCmd *exec.Cmd + decompressorWg sync.WaitGroup + ) srcReaders := []io.Reader{} srcDecompressors := []io.ReadCloser{} + for _, file := range srcFiles { reader := io.Reader(file) // Create the decompressor if needed. if compressed { - decompressor, err := pgzip.NewReader(reader) - if err != nil { - return vterrors.Wrap(err, "can't create gzip decompressor") + var decompressor io.ReadCloser + + switch *decompressMethod { + case "builtin": + logger.Infof("Using built-in decompressor") + + decompressor, err = pgzip.NewReader(reader) + if err != nil { + return vterrors.Wrap(err, "can't create gzip decompressor") + } + case "external": + decompressorFlags := strings.Split(*externalDecompressor, " ") + if len(decompressorFlags) < 1 { + return vterrors.Wrap(err, "external_decompressor is empty") + } + + decompressorCmdPath, err := validateExternalDecompressor(decompressorFlags[0]) + if err != nil { + return vterrors.Wrap(err, "could not validate external decompressor") + } + + decompressorCmd = exec.CommandContext(ctx, decompressorCmdPath, decompressorFlags[1:]...) + decompressorCmd.Stdin = reader + + logger.Infof("Decompressing using %v", decompressorFlags) + + decompressorOut, err := decompressorCmd.StdoutPipe() + if err != nil { + return vterrors.Wrap(err, "cannot create external decompressor stdout pipe") + } + + decompressorErr, err := decompressorCmd.StderrPipe() + if err != nil { + return vterrors.Wrap(err, "cannot create external decompressor stderr pipe") + } + + if err := decompressorCmd.Start(); err != nil { + return vterrors.Wrap(err, "can't start external decompressor") + } + + decompressorWg.Add(1) + go scanLinesToLogger("decompressor stderr", decompressorErr, logger, decompressorWg.Done) + + decompressor = decompressorOut + + defer func() { + decompressorWg.Wait() + // log the exit status + if err := decompressorCmd.Wait(); err != nil { + vterrors.Wrap(err, "external decompressor failed") + } + }() + default: + return vterrors.Wrap(err, "unknown decompressor method") } + srcDecompressors = append(srcDecompressors, decompressor) reader = decompressor } @@ -824,6 +885,15 @@ func (be *XtrabackupEngine) ShouldDrainForBackup() bool { return false } +// Validates if the external decompressor exists and return its path. +func validateExternalDecompressor(cmd string) (string, error) { + if cmd == "" { + return "", errors.New("external compressor command is empty") + } + + return exec.LookPath(cmd) +} + func init() { BackupRestoreEngineMap[xtrabackupEngineName] = &XtrabackupEngine{} } diff --git a/go/vt/mysqlctl/xtrabackupengine_test.go b/go/vt/mysqlctl/xtrabackupengine_test.go index 26e53c6c949..b327d60cbd9 100644 --- a/go/vt/mysqlctl/xtrabackupengine_test.go +++ b/go/vt/mysqlctl/xtrabackupengine_test.go @@ -18,8 +18,10 @@ package mysqlctl import ( "bytes" + "fmt" "io" "math/rand" + "strings" "testing" "vitess.io/vitess/go/vt/logutil" @@ -115,3 +117,40 @@ func TestStripeRoundTrip(t *testing.T) { // Test block size and stripe count that don't evenly divide data size. test(6000, 7) } + +func TestValidateExternalDecompressor(t *testing.T) { + tests := []struct { + cmdName string + path string + errStr string + }{ + // this should not find an executable + {"non_existent_cmd", "", "executable file not found"}, + // we expect ls to be on PATH as it is a basic command part of busybox and most containers + {"ls", "ls", ""}, + } + + for i, tt := range tests { + t.Run(fmt.Sprintf("Test #%d", i+1), func(t *testing.T) { + CmdName := tt.cmdName + + path, err := validateExternalDecompressor(CmdName) + + if tt.path != "" { + if !strings.HasSuffix(path, tt.path) { + t.Errorf("Expected path \"%s\" to include \"%s\"", path, tt.path) + } + } + + if tt.errStr == "" { + if err != nil { + t.Errorf("Expected result \"%v\", got \"%v\"", "", err) + } + } else { + if !strings.Contains(fmt.Sprintf("%v", err), tt.errStr) { + t.Errorf("Expected result \"%v\", got \"%v\"", tt.errStr, err) + } + } + }) + } +} From 0a0040128065b5390895c41e417ec1ad61bd51ff Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Thu, 3 Jun 2021 09:13:38 -0700 Subject: [PATCH 02/21] add external compressor support + builtin additional compressors Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go.mod | 11 +- go.sum | 26 ++- go/vt/mysqlctl/compression.go | 254 ++++++++++++++++++++++++ go/vt/mysqlctl/compression_test.go | 204 +++++++++++++++++++ go/vt/mysqlctl/xtrabackupengine.go | 134 ++++++------- go/vt/mysqlctl/xtrabackupengine_test.go | 39 ---- 6 files changed, 538 insertions(+), 130 deletions(-) create mode 100644 go/vt/mysqlctl/compression.go create mode 100644 go/vt/mysqlctl/compression_test.go diff --git a/go.mod b/go.mod index ae9821f40c0..c3cbbc0189d 100644 --- a/go.mod +++ b/go.mod @@ -26,8 +26,8 @@ require ( github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b github.com/golang/mock v1.5.0 github.com/golang/protobuf v1.5.2 - github.com/golang/snappy v0.0.1 - github.com/google/go-cmp v0.5.6 + github.com/golang/snappy v0.0.3 + github.com/google/go-cmp v0.5.7 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 github.com/google/uuid v1.3.0 github.com/googleapis/gnostic v0.4.1 // indirect @@ -46,7 +46,7 @@ require ( github.com/icrowley/fake v0.0.0-20180203215853-4178557ae428 github.com/imdario/mergo v0.3.12 // indirect github.com/jmespath/go-jmespath v0.4.0 // indirect - github.com/klauspost/compress v1.11.13 // indirect + github.com/klauspost/compress v1.13.0 github.com/klauspost/pgzip v1.2.4 github.com/krishicks/yaml-patch v0.0.10 github.com/magiconair/properties v1.8.5 @@ -63,6 +63,7 @@ require ( github.com/oxtoacart/bpool v0.0.0-20190530202638-03653db5a59c // indirect github.com/patrickmn/go-cache v2.1.0+incompatible github.com/philhofer/fwd v1.0.0 // indirect + github.com/pierrec/lz4 v2.6.1+incompatible github.com/pires/go-proxyproto v0.6.1 github.com/pkg/errors v0.9.1 // indirect github.com/planetscale/pargzip v0.0.0-20201116224723-90c7fc03ea8a @@ -90,6 +91,7 @@ require ( go.etcd.io/etcd/api/v3 v3.5.0 go.etcd.io/etcd/client/pkg/v3 v3.5.0 go.etcd.io/etcd/client/v3 v3.5.0 + golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 golang.org/x/mod v0.5.1 // indirect golang.org/x/net v0.0.0-20211015210444-4f30a5c0130f @@ -137,6 +139,7 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/fatih/color v1.9.0 // indirect github.com/felixge/httpsnoop v1.0.1 // indirect + github.com/frankban/quicktest v1.14.3 // indirect github.com/go-logr/logr v0.2.0 // indirect github.com/gogo/protobuf v1.3.2 // indirect github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect @@ -151,7 +154,6 @@ require ( github.com/jessevdk/go-flags v1.4.0 // indirect github.com/json-iterator/go v1.1.11 // indirect github.com/jstemmer/go-junit-report v0.9.1 // indirect - github.com/kr/pretty v0.2.1 // indirect github.com/mattn/go-colorable v0.1.6 // indirect github.com/mattn/go-ieproxy v0.0.0-20190702010315-6dee0af9227d // indirect github.com/mattn/go-isatty v0.0.12 // indirect @@ -181,7 +183,6 @@ require ( go.uber.org/atomic v1.7.0 // indirect go.uber.org/multierr v1.6.0 // indirect go.uber.org/zap v1.17.0 // indirect - golang.org/x/crypto v0.0.0-20210817164053-32db794688a5 // indirect golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 // indirect google.golang.org/appengine v1.6.7 // indirect gopkg.in/inf.v0 v0.9.1 // indirect diff --git a/go.sum b/go.sum index 5ce015a58c3..d3a99ff84d2 100644 --- a/go.sum +++ b/go.sum @@ -179,6 +179,7 @@ github.com/corpix/uarand v0.1.1/go.mod h1:SFKZvkcRoLqVRFZ4u25xPmp6m9ktANfbpXZ7SJ github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE= github.com/cpuguy83/go-md2man/v2 v2.0.0/go.mod h1:maD7wRr/U5Z6m/iR4s+kqSMx2CaBsrgA7czyZG/E6dU= github.com/creack/pty v1.1.7/go.mod h1:lj5s0c3V2DBrqTV7llrYr5NG6My20zk30Fl46Y7DoTY= +github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyphar/filepath-securejoin v0.2.3 h1:YX6ebbZCZP7VkM3scTTokDgBL2TY741X51MTk3ycuNI= github.com/cyphar/filepath-securejoin v0.2.3/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/daaku/go.zipexe v1.0.0 h1:VSOgZtH418pH9L16hC/JrgSNJbbAL26pj7lmD1+CGdY= @@ -218,6 +219,8 @@ github.com/felixge/httpsnoop v1.0.1 h1:lvB5Jl89CsZtGIWuTcDM1E/vkVs49/Ml7JJe07l8S github.com/felixge/httpsnoop v1.0.1/go.mod h1:m8KPJKqk1gH5J9DgRY2ASl2lWCfGKXixSwevea8zH2U= github.com/form3tech-oss/jwt-go v3.2.2+incompatible h1:TcekIExNqud5crz4xD2pavyTgWiPvpYe4Xau31I0PRk= github.com/form3tech-oss/jwt-go v3.2.2+incompatible/go.mod h1:pbq4aXjuKjdthFRnoDwaVPLA+WlJuPGy+QneDUgJi2k= +github.com/frankban/quicktest v1.14.3 h1:FJKSZTDHjyhriyC81FLQ0LY93eSai0ZyR/ZIkd3ZUKE= +github.com/frankban/quicktest v1.14.3/go.mod h1:mgiwOwqx65TmIk1wJ6Q7wvnVMocbUorkibMOrVTHZps= github.com/fsnotify/fsnotify v1.4.7/go.mod h1:jwhsz4b93w/PPRr/qN1Yymfu8t87LnFCMoQvtojpjFo= github.com/fsnotify/fsnotify v1.4.9 h1:hsms1Qyu0jgnwNXIxa+/V/PDsU6CfLf6CNO8H7IWoS4= github.com/fsnotify/fsnotify v1.4.9/go.mod h1:znqG4EE+3YCdAaPaxE2ZRY/06pZUdp0tY4IgpuI1SZQ= @@ -327,8 +330,8 @@ github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaS github.com/golang/protobuf v1.5.1/go.mod h1:DopwsBzvsk0Fs44TXzsVbJyPhcCPeIwnvohx4u74HPM= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= -github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= +github.com/golang/snappy v0.0.3 h1:fHPg5GQYlCeLIPB9BZqMVR5nR9A+IM5zcgeTdjMYmLA= +github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= github.com/google/btree v1.0.0 h1:0udJVsspx3VBr5FwtLhQQtuAsVc79tTq0ocGIPAU6qo= github.com/google/btree v1.0.0/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= @@ -343,8 +346,8 @@ github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/ github.com/google/go-cmp v0.5.3/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= -github.com/google/go-cmp v0.5.6 h1:BKbKCqvP6I+rmFHt06ZmyQtvB8xAkWdhFyr0ZUNZcxQ= -github.com/google/go-cmp v0.5.6/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.7 h1:81/ik6ipDQS2aGcBfIN5dHDB36BwrStyeAQquSYCV4o= +github.com/google/go-cmp v0.5.7/go.mod h1:n+brtR0CgQNWTVd5ZUFpTBC8YFBDLK/h/bpaJ8/DtOE= github.com/google/go-github/v27 v27.0.4/go.mod h1:/0Gr8pJ55COkmv+S/yPKCczSkUPIM/LnFyubufRNIS0= github.com/google/go-querystring v1.0.0/go.mod h1:odCYkC5MyYFN7vkCjXpyrEuKhc/BUO6wN/zVPAxq5ck= github.com/google/gofuzz v1.0.0/go.mod h1:dBl0BpW6vV/+mYPU4Po3pmUjxk6FQPldtuIdl/M65Eg= @@ -490,8 +493,8 @@ github.com/julienschmidt/httprouter v1.3.0/go.mod h1:JR6WtHb+2LUe8TCKY3cZOxFyyO8 github.com/kisielk/errcheck v1.1.0/go.mod h1:EZBBE59ingxPouuu3KfxchcWSUPOHkagtvWXihfKN4Q= github.com/kisielk/errcheck v1.5.0/go.mod h1:pFxgyoBC7bSaBwPgfKdkLd5X25qrDl4LWUI2bnpBCr8= github.com/kisielk/gotool v1.0.0/go.mod h1:XhKaO+MFFWcvkIS/tQcRk01m1F5IRFswLeQ+oQHNcck= -github.com/klauspost/compress v1.11.13 h1:eSvu8Tmq6j2psUJqJrLcWH6K3w5Dwc+qipbaA6eVEN4= -github.com/klauspost/compress v1.11.13/go.mod h1:aoV0uJVorq1K+umq18yTdKaF57EivdYsUV+/s2qKfXs= +github.com/klauspost/compress v1.13.0 h1:2T7tUoQrQT+fQWdaY5rjWztFGAFwbGD04iPJg90ZiOs= +github.com/klauspost/compress v1.13.0/go.mod h1:8dP1Hq4DHOhN9w426knH3Rhby4rFm6D8eO+e+Dq5Gzg= github.com/klauspost/pgzip v1.2.4 h1:TQ7CNpYKovDOmqzRHKxJh0BeaBI7UdQZYc6p7pMQh1A= github.com/klauspost/pgzip v1.2.4/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs= github.com/konsorten/go-windows-terminal-sequences v1.0.1/go.mod h1:T0+1ngSBFLxvqU3pZ+m/2kptfBszLMUkC4ZK/EgS/cQ= @@ -500,12 +503,13 @@ github.com/kr/fs v0.1.0/go.mod h1:FFnZGqtBN9Gxj7eW1uZ42v5BccTP0vu6NEaFoC2HwRg= github.com/kr/logfmt v0.0.0-20140226030751-b84e30acd515/go.mod h1:+0opPa2QZZtGFBFZlji/RkVcI2GknAs/DXo4wKdlNEc= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pretty v0.2.0/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.2.1 h1:Fmg33tUaq4/8ym9TJN1x7sLJnHVwhP33CNkpYV/7rwI= -github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= +github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= +github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/pty v1.1.5/go.mod h1:9r2w37qlBe7rQ6e1fg1S/9xpWHSnaqNdHD3WcMdbPDA= -github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= +github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/krishicks/yaml-patch v0.0.10 h1:H4FcHpnNwVmw8u0MjPRjWyIXtco6zM2F78t+57oNM3E= github.com/krishicks/yaml-patch v0.0.10/go.mod h1:Sm5TchwZS6sm7RJoyg87tzxm2ZcKzdRE4Q7TjNhPrME= github.com/magiconair/properties v1.8.0/go.mod h1:PppfXfuXeibc/6YijjN8zIbojt8czPbwD3XqdrwzmxQ= @@ -615,6 +619,8 @@ github.com/pelletier/go-toml v1.9.3/go.mod h1:u1nR/EPcESfeI/szUZKdtJ0xRNbUoANCko github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/philhofer/fwd v1.0.0 h1:UbZqGr5Y38ApvM/V/jEljVxwocdweyH+vmYvRPBnbqQ= github.com/philhofer/fwd v1.0.0/go.mod h1:gk3iGcWd9+svBvR0sR+KPcfE+RNWozjowpeBVG3ZVNU= +github.com/pierrec/lz4 v2.6.1+incompatible h1:9UY3+iC23yxF0UfGaYrGplQ+79Rg+h/q9FV9ix19jjM= +github.com/pierrec/lz4 v2.6.1+incompatible/go.mod h1:pdkljMzZIN41W+lC3N2tnIh5sFi+IEE17M5jbnwPHcY= github.com/pires/go-proxyproto v0.6.1 h1:EBupykFmo22SDjv4fQVQd2J9NOoLPmyZA/15ldOGkPw= github.com/pires/go-proxyproto v0.6.1/go.mod h1:Odh9VFOZJCf9G8cLW5o435Xf1J95Jw9Gw5rnCjcwzAY= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -664,6 +670,8 @@ github.com/rcrowley/go-metrics v0.0.0-20200313005456-10cdbea86bc0/go.mod h1:bCqn github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg= github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ= github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4= +github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBOAvL+k= +github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g= github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/ryanuber/columnize v0.0.0-20160712163229-9b3edd62028f/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go new file mode 100644 index 00000000000..bd19cd056c3 --- /dev/null +++ b/go/vt/mysqlctl/compression.go @@ -0,0 +1,254 @@ +package mysqlctl + +import ( + "context" + "errors" + "flag" + "fmt" + "io" + "io/ioutil" + "os/exec" + "strings" + "sync" + + "github.com/klauspost/compress/zstd" + "github.com/klauspost/pgzip" + "github.com/pierrec/lz4" + "github.com/planetscale/pargzip" + + "vitess.io/vitess/go/vt/logutil" + "vitess.io/vitess/go/vt/vterrors" +) + +var ( + compressionLevel = flag.Int("compression_level", 1, "What level to pass to the compressor") + + errUnsupportedCompressionEngine = errors.New("unsupported engine") + errUnsupportedCompressionExtension = errors.New("unsupported extension") + + // this is use by getEngineFromExtension() to figure out which engine to use in case the user didn't specify + engineExtensions = map[string][]string{ + ".gz": {"pgzip", "pargzip"}, + ".lz4": {"lz4"}, + ".zst": {"zstd"}, + } +) + +func getEngineFromExtension(extension string) (string, error) { + for ext, eng := range engineExtensions { + if ext == extension { + return eng[0], nil // we select the first supported engine in auto mode + } + } + + return "", fmt.Errorf("%w \"%s\"", errUnsupportedCompressionExtension, extension) +} + +func getExtensionFromEngine(engine string) (string, error) { + for ext, eng := range engineExtensions { + for _, e := range eng { + if e == engine { + return ext, nil + } + } + } + + return "", fmt.Errorf("%w \"%s\"", errUnsupportedCompressionEngine, engine) +} + +// Validates if the external decompressor exists and return its path. +func validateExternalCmd(cmd string) (string, error) { + if cmd == "" { + return "", errors.New("external command is empty") + } + + return exec.LookPath(cmd) +} + +func prepareExternalCompressionCmd(ctx context.Context, cmdStr string) (*exec.Cmd, error) { + cmdArgs := strings.Split(cmdStr, " ") + if len(cmdArgs) < 1 { + return nil, errors.New("external command is empty") + } + + cmdPath, err := validateExternalCmd(cmdArgs[0]) + if err != nil { + return nil, err + } + + return exec.CommandContext(ctx, cmdPath, cmdArgs[1:]...), nil +} + +// This returns a writer that writes the compressed output of the external command to the provided writer. +// It is important to wait on the WaitGroup provided to make sure that even after closing the writer, +// the command will have processed its input buffer, otherwise not all data might have been written to the target writer. +func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, wg *sync.WaitGroup, logger logutil.Logger) (compressor io.WriteCloser, err error) { + logger.Infof("Compressing using external command: \"%s\"", cmdStr) + + cmd, err := prepareExternalCompressionCmd(ctx, cmdStr) + if err != nil { + return compressor, vterrors.Wrap(err, "unable to start external command") + } + + cmd.Stdout = writer + + cmdIn, err := cmd.StdinPipe() + if err != nil { + return compressor, vterrors.Wrap(err, "cannot create external ompressor stdin pipe") + } + + cmdErr, err := cmd.StderrPipe() + if err != nil { + return compressor, vterrors.Wrap(err, "cannot create external ompressor stderr pipe") + } + + if err := cmd.Start(); err != nil { + return compressor, vterrors.Wrap(err, "can't start external decompressor") + } + + compressor = cmdIn + + wg.Add(2) // one for the logger, another one for the go func below + go scanLinesToLogger("compressor stderr", cmdErr, logger, wg.Done) + + go func() { + // log the exit status + if err := cmd.Wait(); err != nil { + logger.Errorf("external compressor failed: %v", err) + } + wg.Done() + }() + + return +} + +// This returns a reader that reads the compressed input and passes it to the external command to be decompressed. Calls to its +// Read() will return the uncompressed data until EOF. +func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { + var decompressorWg sync.WaitGroup + + logger.Infof("Decompressing using external command: \"%s\"", cmdStr) + + cmd, err := prepareExternalCompressionCmd(ctx, cmdStr) + if err != nil { + return decompressor, vterrors.Wrap(err, "unable to start external command") + } + + cmd.Stdin = reader + + cmdOut, err := cmd.StdoutPipe() + if err != nil { + return decompressor, vterrors.Wrap(err, "cannot create external decompressor stdout pipe") + } + + cmdErr, err := cmd.StderrPipe() + if err != nil { + return decompressor, vterrors.Wrap(err, "cannot create external decompressor stderr pipe") + } + + if err := cmd.Start(); err != nil { + return decompressor, vterrors.Wrap(err, "can't start external decompressor") + } + + decompressorWg.Add(1) + go scanLinesToLogger("decompressor stderr", cmdErr, logger, decompressorWg.Done) + + decompressor = cmdOut + + go func() { + decompressorWg.Wait() + // log the exit status + if err := cmd.Wait(); err != nil { + logger.Errorf("external compressor failed: %v", err) + } + }() + + return +} + +// This returns a reader that will decompress the underlying provided reader and will use the specified supported engine (or +// try to detect which one to use based on the extension if the default "auto" is used. +func newBuiltinDecompressor(engine, extension string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { + if engine == "auto" { + logger.Infof("Builtin decompressor set to auto, checking which engine to decompress based on the extension") + + eng, err := getEngineFromExtension(extension) + if err != nil { + return decompressor, err + } + + engine = eng + } + + switch engine { + case "pgzip": + d, err := pgzip.NewReader(reader) + if err != nil { + return nil, err + } + decompressor = d + case "pargzip": + err = errors.New("engine pargzip does not support decompression") + return decompressor, err + case "lz4": + decompressor = ioutil.NopCloser(lz4.NewReader(reader)) + case "zstd": + d, err := zstd.NewReader(reader) + if err != nil { + return nil, err + } + + decompressor = d.IOReadCloser() + default: + err = fmt.Errorf("Unkown decompressor engine: \"%s\"", engine) + return decompressor, err + } + + logger.Infof("Decompressing backup using built-in engine \"%s\"", engine) + + return decompressor, err +} + +// This return a writer that will compress the data using the specified engine before writing to the underlying writer. +func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger) (compressor io.WriteCloser, err error) { + switch engine { + case "pgzip": + gzip, err := pgzip.NewWriterLevel(writer, *compressionLevel) + if err != nil { + return compressor, vterrors.Wrap(err, "cannot create gzip compressor") + } + + gzip.SetConcurrency(*backupCompressBlockSize, *backupCompressBlocks) + + compressor = gzip + case "pargzip": + gzip := pargzip.NewWriter(writer) + gzip.ChunkSize = *backupCompressBlockSize + gzip.Parallel = *backupCompressBlocks + gzip.CompressionLevel = *compressionLevel + + compressor = gzip + case "lz4": + lz4Writer := lz4.NewWriter(writer).WithConcurrency(*backupCompressBlocks) + lz4Writer.Header = lz4.Header{ + CompressionLevel: *compressionLevel, + } + + compressor = lz4Writer + case "zstd": + // compressor = zstd.NewWriterLevel(writer, *compressionLevel) + zst, err := zstd.NewWriter(writer, zstd.WithEncoderLevel(zstd.EncoderLevel(*compressionLevel))) + if err != nil { + return compressor, vterrors.Wrap(err, "cannot create zstd compressor") + } + + compressor = zst + default: + err = fmt.Errorf("Unkown compressor engine: \"%s\"", engine) + return compressor, err + } + + logger.Infof("Compressing backup using built-in engine \"%s\"", engine) + + return +} diff --git a/go/vt/mysqlctl/compression_test.go b/go/vt/mysqlctl/compression_test.go new file mode 100644 index 00000000000..54cde3d3421 --- /dev/null +++ b/go/vt/mysqlctl/compression_test.go @@ -0,0 +1,204 @@ +package mysqlctl + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "reflect" + "strings" + "sync" + "testing" + "time" + + "vitess.io/vitess/go/vt/logutil" +) + +func TestGetExtensionFromEngine(t *testing.T) { + tests := []struct { + engine, extension string + err error + }{ + {"pgzip", ".gz", nil}, + {"pargzip", ".gz", nil}, + {"lz4", ".lz4", nil}, + {"zstd", ".zst", nil}, + {"foobar", "", errUnsupportedCompressionEngine}, + } + + for _, tt := range tests { + t.Run(tt.engine, func(t *testing.T) { + ext, err := getExtensionFromEngine(tt.engine) + // if err != tt.err { + if !errors.Is(err, tt.err) { + t.Errorf("got err: %v; expected: %v", err, tt.err) + } + // } + + if ext != tt.extension { + t.Errorf("got err: %v; expected: %v", ext, tt.extension) + } + }) + } +} + +func TestBuiltinCompressors(t *testing.T) { + data := []byte("foo bar foobar") + + logger := logutil.NewMemoryLogger() + + for _, engine := range []string{"pgzip", "lz4", "zstd"} { + t.Run(engine, func(t *testing.T) { + var compressed, decompressed bytes.Buffer + + reader := bytes.NewReader(data) + + compressor, err := newBuiltinCompressor(engine, &compressed, logger) + if err != nil { + t.Fatal(err) + } + + _, err = io.Copy(compressor, reader) + if err != nil { + t.Error(err) + return + } + compressor.Close() + + decompressor, err := newBuiltinDecompressor(engine, "not used", &compressed, logger) + if err != nil { + t.Error(err) + return + } + + _, err = io.Copy(&decompressed, decompressor) + if err != nil { + t.Error(err) + return + } + decompressor.Close() + + if len(data) != len(decompressed.Bytes()) { + t.Errorf("Different size of original (%d bytes) and uncompressed (%d bytes) data", len(data), len(decompressed.Bytes())) + } + + if !reflect.DeepEqual(data, decompressed.Bytes()) { + t.Error("decompressed content differs from the original") + } + }) + } +} + +func TestExternalCompressors(t *testing.T) { + data := []byte("foo bar foobar") + logger := logutil.NewMemoryLogger() + + tests := []struct { + compress, decompress string + }{ + {"gzip", "gzip -d"}, + {"pigz", "pigz -d"}, + {"lz4", "lz4 -d"}, + {"zstd", "zstd -d"}, + {"lzop", "lzop -d"}, + {"bzip2", "bzip2 -d"}, + {"lzma", "lzma -d"}, + } + + for _, tt := range tests { + t.Run(tt.compress, func(t *testing.T) { + var ( + compressed, decompressed bytes.Buffer + wg sync.WaitGroup + ) + + reader := bytes.NewReader(data) + + for _, cmd := range []string{tt.compress, tt.decompress} { + cmdArgs := strings.Split(cmd, " ") + + _, err := validateExternalCmd(cmdArgs[0]) + if err != nil { + t.Skip("Command not available in this host:", err) + } + } + + ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) + defer cancel() + + compressor, err := newExternalCompressor(ctx, tt.compress, &compressed, &wg, logger) + if err != nil { + t.Error(err) + return + } + + _, err = io.Copy(compressor, reader) + if err != nil { + t.Error(err) + return + } + compressor.Close() + wg.Wait() + + decompressor, err := newExternalDecompressor(ctx, tt.decompress, &compressed, logger) + if err != nil { + t.Error(err) + return + } + + _, err = io.Copy(&decompressed, decompressor) + if err != nil { + t.Error(err) + return + } + decompressor.Close() + + if len(data) != len(decompressed.Bytes()) { + t.Errorf("Different size of original (%d bytes) and uncompressed (%d bytes) data", len(data), len(decompressed.Bytes())) + } + + if !reflect.DeepEqual(data, decompressed.Bytes()) { + t.Error("decompressed content differs from the original") + } + + }) + } +} + +func TestValidateExternalCmd(t *testing.T) { + tests := []struct { + cmdName string + path string + errStr string + }{ + // this should not find an executable + {"non_existent_cmd", "", "executable file not found"}, + // we expect ls to be on PATH as it is a basic command part of busybox and most containers + {"ls", "ls", ""}, + } + + for i, tt := range tests { + t.Run(fmt.Sprintf("Test #%d", i+1), func(t *testing.T) { + CmdName := tt.cmdName + + path, err := validateExternalCmd(CmdName) + + if tt.path != "" { + if !strings.HasSuffix(path, tt.path) { + t.Errorf("Expected path \"%s\" to include \"%s\"", path, tt.path) + } + } + + if tt.errStr == "" { + if err != nil { + t.Errorf("Expected result \"%v\", got \"%v\"", "", err) + } + } else { + if !strings.Contains(fmt.Sprintf("%v", err), tt.errStr) { + t.Errorf("Expected result \"%v\", got \"%v\"", tt.errStr, err) + } + } + }) + } +} diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 45bf6d85e01..31a733cad84 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -20,21 +20,18 @@ import ( "bufio" "context" "encoding/json" - "errors" "flag" "fmt" "io" "os" "os/exec" "path" + "path/filepath" "regexp" "strings" "sync" "time" - "github.com/klauspost/pgzip" - "github.com/planetscale/pargzip" - "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/vt/logutil" "vitess.io/vitess/go/vt/mysqlctl/backupstorage" @@ -64,9 +61,13 @@ var ( // striping mode xtrabackupStripes = flag.Uint("xtrabackup_stripes", 0, "If greater than 0, use data striping across this many destination files to parallelize data transfer and decompression") xtrabackupStripeBlockSize = flag.Uint("xtrabackup_stripe_block_size", 102400, "Size in bytes of each block that gets sent to a given stripe before rotating to the next stripe") + // switch which compressor/decompressor to use + builtinCompressor = flag.String("xtrabackup_builtin_compressor", "pgzip", "which builtin compressor engine to use") + builtinDecompressor = flag.String("xtrabackup_builtin_decompressor", "auto", "which builtin decompressor engine to use") // use and external command to decompress the backups - decompressMethod = flag.String("decompress_method", "builtin", "what decompressor to use [builtin|external]") - externalDecompressor = flag.String("external_decompressor", "", "command with arguments to which decompressor to run") + externalCompressor = flag.String("xtrabackup_external_compressor", "", "command with arguments to use when decompressing a backup") + externalCompressorExt = flag.String("xtrabackup_external_compressor_extension", "", "which extension to use when using an external decompressor") + externalDecompressor = flag.String("xtrabackup_external_decompressor", "", "command with arguments to use when compressing a backup") ) const ( @@ -112,7 +113,16 @@ func (be *XtrabackupEngine) backupFileName() string { fileName += *xtrabackupStreamMode } if *backupStorageCompress { - fileName += ".gz" + if *externalDecompressor != "" { + fileName += *externalCompressorExt + } else { + if ext, err := getExtensionFromEngine(*builtinCompressor); err != nil { + // there is a check for this, but just in case that fails, we set a extension to the file + fileName += ".unknown" + } else { + fileName += ext + } + } } return fileName } @@ -134,6 +144,13 @@ func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupPara if *xtrabackupUser == "" { return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "xtrabackupUser must be specified.") } + + // an extension is required when using an external compressor + if *backupStorageCompress && *externalCompressor != "" && *externalCompressorExt == "" { + return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, + "xtrabackup_external_compressor_extension not provided when using an external compressor") + } + // use a mysql connection to detect flavor at runtime conn, err := params.Mysqld.GetDbaConnection(ctx) if conn != nil && err == nil { @@ -151,6 +168,7 @@ func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupPara params.Logger.Infof("Detected MySQL flavor: %v", flavor) backupFileName := be.backupFileName() + params.Logger.Infof("backup file name: %s", backupFileName) numStripes := int(*xtrabackupStripes) // Perform backups in a separate function, so deferred calls to Close() are @@ -273,6 +291,7 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams return replicationPosition, vterrors.Wrap(err, "cannot create stderr pipe") } + var extCompressorWaitGroup sync.WaitGroup destWriters := []io.Writer{} destBuffers := []*bufio.Writer{} destCompressors := []io.WriteCloser{} @@ -283,10 +302,20 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams // Create the gzip compression pipe, if necessary. if *backupStorageCompress { - compressor := pargzip.NewWriter(writer) - compressor.ChunkSize = *backupCompressBlockSize - compressor.Parallel = *backupCompressBlocks - compressor.CompressionLevel = pargzip.BestSpeed + var compressor io.WriteCloser + + if *externalCompressor != "" { + compressor, err = newExternalCompressor(ctx, *externalCompressor, writer, &extCompressorWaitGroup, params.Logger) + if err != nil { + return replicationPosition, err + } + } else { + compressor, err = newBuiltinCompressor(*builtinCompressor, writer, params.Logger) + if err != nil { + return replicationPosition, err + } + } + writer = compressor destCompressors = append(destCompressors, compressor) } @@ -351,6 +380,10 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams } } + // Wait for any external compressor to finish processing their current input buffer and exit. + // If we skip this, we may not send the entire data when we flush below and can corrupt the backup + extCompressorWaitGroup.Wait() + // Flush the buffer to finish writing on destination. for _, buffer := range destBuffers { if err = buffer.Flush(); err != nil { @@ -514,6 +547,9 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log baseFileName = be.backupFileName() } + logger.Infof("backup file name: %s", baseFileName) + extension := filepath.Ext(baseFileName) + // Open the source files for reading. srcFiles, err := readStripeFiles(ctx, bh, baseFileName, int(bm.NumStripes), logger) if err != nil { @@ -525,12 +561,7 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log } }() - var ( - decompressorCmd *exec.Cmd - decompressorWg sync.WaitGroup - ) srcReaders := []io.Reader{} - srcDecompressors := []io.ReadCloser{} for _, file := range srcFiles { reader := io.Reader(file) @@ -539,73 +570,31 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log if compressed { var decompressor io.ReadCloser - switch *decompressMethod { - case "builtin": - logger.Infof("Using built-in decompressor") - - decompressor, err = pgzip.NewReader(reader) - if err != nil { - return vterrors.Wrap(err, "can't create gzip decompressor") - } - case "external": - decompressorFlags := strings.Split(*externalDecompressor, " ") - if len(decompressorFlags) < 1 { - return vterrors.Wrap(err, "external_decompressor is empty") - } - - decompressorCmdPath, err := validateExternalDecompressor(decompressorFlags[0]) - if err != nil { - return vterrors.Wrap(err, "could not validate external decompressor") - } - - decompressorCmd = exec.CommandContext(ctx, decompressorCmdPath, decompressorFlags[1:]...) - decompressorCmd.Stdin = reader - - logger.Infof("Decompressing using %v", decompressorFlags) - - decompressorOut, err := decompressorCmd.StdoutPipe() + if *externalDecompressor != "" { + decompressor, err = newExternalDecompressor(ctx, *externalDecompressor, reader, logger) if err != nil { - return vterrors.Wrap(err, "cannot create external decompressor stdout pipe") + return err } - - decompressorErr, err := decompressorCmd.StderrPipe() + } else { + decompressor, err = newBuiltinDecompressor(*builtinDecompressor, extension, reader, logger) if err != nil { - return vterrors.Wrap(err, "cannot create external decompressor stderr pipe") + return err } - if err := decompressorCmd.Start(); err != nil { - return vterrors.Wrap(err, "can't start external decompressor") - } - - decompressorWg.Add(1) - go scanLinesToLogger("decompressor stderr", decompressorErr, logger, decompressorWg.Done) - - decompressor = decompressorOut - + // we only need to close the builtin decompressors, because the externals will be automatically + // closed by the go func inside newExternalDecompressor defer func() { - decompressorWg.Wait() - // log the exit status - if err := decompressorCmd.Wait(); err != nil { - vterrors.Wrap(err, "external decompressor failed") + if cerr := decompressor.Close(); cerr != nil { + logger.Errorf("failed to close gzip decompressor: %v", cerr) } }() - default: - return vterrors.Wrap(err, "unknown decompressor method") } - srcDecompressors = append(srcDecompressors, decompressor) reader = decompressor } srcReaders = append(srcReaders, reader) } - defer func() { - for _, decompressor := range srcDecompressors { - if cerr := decompressor.Close(); cerr != nil { - logger.Errorf("failed to close gzip decompressor: %v", cerr) - } - } - }() reader := stripeReader(srcReaders, int64(bm.StripeBlockSize)) @@ -885,15 +874,6 @@ func (be *XtrabackupEngine) ShouldDrainForBackup() bool { return false } -// Validates if the external decompressor exists and return its path. -func validateExternalDecompressor(cmd string) (string, error) { - if cmd == "" { - return "", errors.New("external compressor command is empty") - } - - return exec.LookPath(cmd) -} - func init() { BackupRestoreEngineMap[xtrabackupEngineName] = &XtrabackupEngine{} } diff --git a/go/vt/mysqlctl/xtrabackupengine_test.go b/go/vt/mysqlctl/xtrabackupengine_test.go index b327d60cbd9..26e53c6c949 100644 --- a/go/vt/mysqlctl/xtrabackupengine_test.go +++ b/go/vt/mysqlctl/xtrabackupengine_test.go @@ -18,10 +18,8 @@ package mysqlctl import ( "bytes" - "fmt" "io" "math/rand" - "strings" "testing" "vitess.io/vitess/go/vt/logutil" @@ -117,40 +115,3 @@ func TestStripeRoundTrip(t *testing.T) { // Test block size and stripe count that don't evenly divide data size. test(6000, 7) } - -func TestValidateExternalDecompressor(t *testing.T) { - tests := []struct { - cmdName string - path string - errStr string - }{ - // this should not find an executable - {"non_existent_cmd", "", "executable file not found"}, - // we expect ls to be on PATH as it is a basic command part of busybox and most containers - {"ls", "ls", ""}, - } - - for i, tt := range tests { - t.Run(fmt.Sprintf("Test #%d", i+1), func(t *testing.T) { - CmdName := tt.cmdName - - path, err := validateExternalDecompressor(CmdName) - - if tt.path != "" { - if !strings.HasSuffix(path, tt.path) { - t.Errorf("Expected path \"%s\" to include \"%s\"", path, tt.path) - } - } - - if tt.errStr == "" { - if err != nil { - t.Errorf("Expected result \"%v\", got \"%v\"", "", err) - } - } else { - if !strings.Contains(fmt.Sprintf("%v", err), tt.errStr) { - t.Errorf("Expected result \"%v\", got \"%v\"", tt.errStr, err) - } - } - }) - } -} From c71fd0a72d64a8af9f1bf7db5c12f6743c01ed12 Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Wed, 9 Jun 2021 10:17:21 -0700 Subject: [PATCH 03/21] wrap external compressor/decompressor Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/compression.go | 126 ++++++++++++++++++----------- go/vt/mysqlctl/compression_test.go | 9 +-- go/vt/mysqlctl/xtrabackupengine.go | 61 ++++++-------- 3 files changed, 105 insertions(+), 91 deletions(-) diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index bd19cd056c3..06c86b6181e 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -8,9 +8,9 @@ import ( "io" "io/ioutil" "os/exec" - "strings" "sync" + "github.com/google/shlex" "github.com/klauspost/compress/zstd" "github.com/klauspost/pgzip" "github.com/pierrec/lz4" @@ -41,7 +41,7 @@ func getEngineFromExtension(extension string) (string, error) { } } - return "", fmt.Errorf("%w \"%s\"", errUnsupportedCompressionExtension, extension) + return "", fmt.Errorf("%w %q", errUnsupportedCompressionExtension, extension) } func getExtensionFromEngine(engine string) (string, error) { @@ -53,7 +53,7 @@ func getExtensionFromEngine(engine string) (string, error) { } } - return "", fmt.Errorf("%w \"%s\"", errUnsupportedCompressionEngine, engine) + return "", fmt.Errorf("%w %q", errUnsupportedCompressionEngine, engine) } // Validates if the external decompressor exists and return its path. @@ -66,7 +66,11 @@ func validateExternalCmd(cmd string) (string, error) { } func prepareExternalCompressionCmd(ctx context.Context, cmdStr string) (*exec.Cmd, error) { - cmdArgs := strings.Split(cmdStr, " ") + cmdArgs, err := shlex.Split(cmdStr) + if err != nil { + return nil, err + } + if len(cmdArgs) < 1 { return nil, errors.New("external command is empty") } @@ -82,88 +86,73 @@ func prepareExternalCompressionCmd(ctx context.Context, cmdStr string) (*exec.Cm // This returns a writer that writes the compressed output of the external command to the provided writer. // It is important to wait on the WaitGroup provided to make sure that even after closing the writer, // the command will have processed its input buffer, otherwise not all data might have been written to the target writer. -func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, wg *sync.WaitGroup, logger logutil.Logger) (compressor io.WriteCloser, err error) { - logger.Infof("Compressing using external command: \"%s\"", cmdStr) +func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, logger logutil.Logger) (io.WriteCloser, error) { + logger.Infof("Compressing using external command: %q", cmdStr) cmd, err := prepareExternalCompressionCmd(ctx, cmdStr) if err != nil { - return compressor, vterrors.Wrap(err, "unable to start external command") + return nil, vterrors.Wrap(err, "unable to start external command") } + compressor := &externalCompressor{cmd: cmd} cmd.Stdout = writer cmdIn, err := cmd.StdinPipe() if err != nil { - return compressor, vterrors.Wrap(err, "cannot create external ompressor stdin pipe") + return nil, vterrors.Wrap(err, "cannot create external ompressor stdin pipe") } + compressor.stdin = cmdIn + cmdErr, err := cmd.StderrPipe() if err != nil { - return compressor, vterrors.Wrap(err, "cannot create external ompressor stderr pipe") + return nil, vterrors.Wrap(err, "cannot create external ompressor stderr pipe") } if err := cmd.Start(); err != nil { - return compressor, vterrors.Wrap(err, "can't start external decompressor") + return nil, vterrors.Wrap(err, "can't start external decompressor") } - compressor = cmdIn - - wg.Add(2) // one for the logger, another one for the go func below - go scanLinesToLogger("compressor stderr", cmdErr, logger, wg.Done) - - go func() { - // log the exit status - if err := cmd.Wait(); err != nil { - logger.Errorf("external compressor failed: %v", err) - } - wg.Done() - }() + compressor.wg.Add(1) // one for the logger, another one for the go func below + go scanLinesToLogger("compressor stderr", cmdErr, logger, compressor.wg.Done) - return + return compressor, nil } // This returns a reader that reads the compressed input and passes it to the external command to be decompressed. Calls to its // Read() will return the uncompressed data until EOF. -func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { - var decompressorWg sync.WaitGroup - - logger.Infof("Decompressing using external command: \"%s\"", cmdStr) +func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reader, logger logutil.Logger) (io.ReadCloser, error) { + logger.Infof("Decompressing using external command: %q", cmdStr) cmd, err := prepareExternalCompressionCmd(ctx, cmdStr) if err != nil { - return decompressor, vterrors.Wrap(err, "unable to start external command") + return nil, vterrors.Wrap(err, "unable to start external command") } + decompressor := &externalDecompressor{cmd: cmd} + cmd.Stdin = reader cmdOut, err := cmd.StdoutPipe() if err != nil { - return decompressor, vterrors.Wrap(err, "cannot create external decompressor stdout pipe") + return nil, vterrors.Wrap(err, "cannot create external decompressor stdout pipe") } + decompressor.stdout = cmdOut + cmdErr, err := cmd.StderrPipe() if err != nil { - return decompressor, vterrors.Wrap(err, "cannot create external decompressor stderr pipe") + return nil, vterrors.Wrap(err, "cannot create external decompressor stderr pipe") } if err := cmd.Start(); err != nil { - return decompressor, vterrors.Wrap(err, "can't start external decompressor") + return nil, vterrors.Wrap(err, "can't start external decompressor") } - decompressorWg.Add(1) - go scanLinesToLogger("decompressor stderr", cmdErr, logger, decompressorWg.Done) + decompressor.wg.Add(1) + go scanLinesToLogger("decompressor stderr", cmdErr, logger, decompressor.wg.Done) - decompressor = cmdOut - - go func() { - decompressorWg.Wait() - // log the exit status - if err := cmd.Wait(); err != nil { - logger.Errorf("external compressor failed: %v", err) - } - }() - - return + return decompressor, nil } // This returns a reader that will decompress the underlying provided reader and will use the specified supported engine (or @@ -200,11 +189,11 @@ func newBuiltinDecompressor(engine, extension string, reader io.Reader, logger l decompressor = d.IOReadCloser() default: - err = fmt.Errorf("Unkown decompressor engine: \"%s\"", engine) + err = fmt.Errorf("Unkown decompressor engine: %q", engine) return decompressor, err } - logger.Infof("Decompressing backup using built-in engine \"%s\"", engine) + logger.Infof("Decompressing backup using built-in engine %q", engine) return decompressor, err } @@ -244,11 +233,52 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger compressor = zst default: - err = fmt.Errorf("Unkown compressor engine: \"%s\"", engine) + err = fmt.Errorf("Unkown compressor engine: %q", engine) return compressor, err } - logger.Infof("Compressing backup using built-in engine \"%s\"", engine) + logger.Infof("Compressing backup using built-in engine %q", engine) return } + +// This struct wraps the underlying exec.Cmd and implements the io.WriteCloser interface. +type externalCompressor struct { + cmd *exec.Cmd + stdin io.WriteCloser + wg sync.WaitGroup +} + +func (e *externalCompressor) Write(p []byte) (n int, err error) { + return e.stdin.Write(p) +} + +func (e *externalCompressor) Close() error { + if err := e.stdin.Close(); err != nil { + return err + } + + // wait for the stderr to finish reading as well + e.wg.Wait() + + return e.cmd.Wait() +} + +// This struct wraps the underlying exec.Cmd and implements the io.ReadCloser interface. +type externalDecompressor struct { + cmd *exec.Cmd + stdout io.ReadCloser + wg sync.WaitGroup +} + +func (e *externalDecompressor) Read(p []byte) (n int, err error) { + return e.stdout.Read(p) +} + +func (e *externalDecompressor) Close() error { + // wait for the stderr to finish reading as well + e.wg.Wait() + + // exec.Cmd.Wait() will also close the stdout pipe, so we don't need to call it directly + return e.cmd.Wait() +} diff --git a/go/vt/mysqlctl/compression_test.go b/go/vt/mysqlctl/compression_test.go index 54cde3d3421..56d8577c97b 100644 --- a/go/vt/mysqlctl/compression_test.go +++ b/go/vt/mysqlctl/compression_test.go @@ -8,7 +8,6 @@ import ( "io" "reflect" "strings" - "sync" "testing" "time" @@ -108,10 +107,7 @@ func TestExternalCompressors(t *testing.T) { for _, tt := range tests { t.Run(tt.compress, func(t *testing.T) { - var ( - compressed, decompressed bytes.Buffer - wg sync.WaitGroup - ) + var compressed, decompressed bytes.Buffer reader := bytes.NewReader(data) @@ -127,7 +123,7 @@ func TestExternalCompressors(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - compressor, err := newExternalCompressor(ctx, tt.compress, &compressed, &wg, logger) + compressor, err := newExternalCompressor(ctx, tt.compress, &compressed, logger) if err != nil { t.Error(err) return @@ -139,7 +135,6 @@ func TestExternalCompressors(t *testing.T) { return } compressor.Close() - wg.Wait() decompressor, err := newExternalDecompressor(ctx, tt.decompress, &compressed, logger) if err != nil { diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 31a733cad84..b248d2297cc 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -65,9 +65,9 @@ var ( builtinCompressor = flag.String("xtrabackup_builtin_compressor", "pgzip", "which builtin compressor engine to use") builtinDecompressor = flag.String("xtrabackup_builtin_decompressor", "auto", "which builtin decompressor engine to use") // use and external command to decompress the backups - externalCompressor = flag.String("xtrabackup_external_compressor", "", "command with arguments to use when decompressing a backup") - externalCompressorExt = flag.String("xtrabackup_external_compressor_extension", "", "which extension to use when using an external decompressor") - externalDecompressor = flag.String("xtrabackup_external_decompressor", "", "command with arguments to use when compressing a backup") + externalCompressorCmd = flag.String("xtrabackup_external_compressor", "", "command with arguments to use when decompressing a backup") + externalCompressorExt = flag.String("xtrabackup_external_compressor_extension", "", "which extension to use when using an external decompressor") + externalDecompressorCmd = flag.String("xtrabackup_external_decompressor", "", "command with arguments to use when compressing a backup") ) const ( @@ -113,7 +113,7 @@ func (be *XtrabackupEngine) backupFileName() string { fileName += *xtrabackupStreamMode } if *backupStorageCompress { - if *externalDecompressor != "" { + if *externalDecompressorCmd != "" { fileName += *externalCompressorExt } else { if ext, err := getExtensionFromEngine(*builtinCompressor); err != nil { @@ -146,7 +146,7 @@ func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupPara } // an extension is required when using an external compressor - if *backupStorageCompress && *externalCompressor != "" && *externalCompressorExt == "" { + if *backupStorageCompress && *externalCompressorCmd != "" && *externalCompressorExt == "" { return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "xtrabackup_external_compressor_extension not provided when using an external compressor") } @@ -291,7 +291,6 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams return replicationPosition, vterrors.Wrap(err, "cannot create stderr pipe") } - var extCompressorWaitGroup sync.WaitGroup destWriters := []io.Writer{} destBuffers := []*bufio.Writer{} destCompressors := []io.WriteCloser{} @@ -304,16 +303,13 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams if *backupStorageCompress { var compressor io.WriteCloser - if *externalCompressor != "" { - compressor, err = newExternalCompressor(ctx, *externalCompressor, writer, &extCompressorWaitGroup, params.Logger) - if err != nil { - return replicationPosition, err - } + if *externalCompressorCmd != "" { + compressor, err = newExternalCompressor(ctx, *externalCompressorCmd, writer, params.Logger) } else { compressor, err = newBuiltinCompressor(*builtinCompressor, writer, params.Logger) - if err != nil { - return replicationPosition, err - } + } + if err != nil { + return replicationPosition, vterrors.Wrap(err, "can't create compressor") } writer = compressor @@ -376,14 +372,10 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams // Close compressor to flush it. After that all data is sent to the buffer. for _, compressor := range destCompressors { if err := compressor.Close(); err != nil { - return replicationPosition, vterrors.Wrap(err, "cannot close gzip compressor") + return replicationPosition, vterrors.Wrap(err, "cannot close compressor") } } - // Wait for any external compressor to finish processing their current input buffer and exit. - // If we skip this, we may not send the entire data when we flush below and can corrupt the backup - extCompressorWaitGroup.Wait() - // Flush the buffer to finish writing on destination. for _, buffer := range destBuffers { if err = buffer.Flush(); err != nil { @@ -562,7 +554,7 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log }() srcReaders := []io.Reader{} - + srcDecompressors := []io.ReadCloser{} for _, file := range srcFiles { reader := io.Reader(file) @@ -570,31 +562,28 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log if compressed { var decompressor io.ReadCloser - if *externalDecompressor != "" { - decompressor, err = newExternalDecompressor(ctx, *externalDecompressor, reader, logger) - if err != nil { - return err - } + if *externalDecompressorCmd != "" { + decompressor, err = newExternalDecompressor(ctx, *externalDecompressorCmd, reader, logger) } else { decompressor, err = newBuiltinDecompressor(*builtinDecompressor, extension, reader, logger) - if err != nil { - return err - } - - // we only need to close the builtin decompressors, because the externals will be automatically - // closed by the go func inside newExternalDecompressor - defer func() { - if cerr := decompressor.Close(); cerr != nil { - logger.Errorf("failed to close gzip decompressor: %v", cerr) - } - }() + } + if err != nil { + return vterrors.Wrap(err, "can't create decompressor") } + srcDecompressors = append(srcDecompressors, decompressor) reader = decompressor } srcReaders = append(srcReaders, reader) } + defer func() { + for _, decompressor := range srcDecompressors { + if cerr := decompressor.Close(); cerr != nil { + logger.Errorf("failed to close decompressor: %v", cerr) + } + } + }() reader := stripeReader(srcReaders, int64(bm.StripeBlockSize)) From 3c3180fb3f414ca642cd1b5e2536d915b5f4befa Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Thu, 10 Jun 2021 03:08:05 -0700 Subject: [PATCH 04/21] go mod tidy + comments Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/compression.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index 06c86b6181e..a6e03dfced7 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -26,7 +26,7 @@ var ( errUnsupportedCompressionEngine = errors.New("unsupported engine") errUnsupportedCompressionExtension = errors.New("unsupported extension") - // this is use by getEngineFromExtension() to figure out which engine to use in case the user didn't specify + // this is used by getEngineFromExtension() to figure out which engine to use in case the user didn't specify engineExtensions = map[string][]string{ ".gz": {"pgzip", "pargzip"}, ".lz4": {"lz4"}, @@ -84,8 +84,6 @@ func prepareExternalCompressionCmd(ctx context.Context, cmdStr string) (*exec.Cm } // This returns a writer that writes the compressed output of the external command to the provided writer. -// It is important to wait on the WaitGroup provided to make sure that even after closing the writer, -// the command will have processed its input buffer, otherwise not all data might have been written to the target writer. func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, logger logutil.Logger) (io.WriteCloser, error) { logger.Infof("Compressing using external command: %q", cmdStr) @@ -113,7 +111,7 @@ func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, return nil, vterrors.Wrap(err, "can't start external decompressor") } - compressor.wg.Add(1) // one for the logger, another one for the go func below + compressor.wg.Add(1) // we wait for the gorouting to finish when we call Close() on the writer go scanLinesToLogger("compressor stderr", cmdErr, logger, compressor.wg.Done) return compressor, nil @@ -149,7 +147,7 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade return nil, vterrors.Wrap(err, "can't start external decompressor") } - decompressor.wg.Add(1) + decompressor.wg.Add(1) // we wait for the gorouting to finish when we call Close() on the reader go scanLinesToLogger("decompressor stderr", cmdErr, logger, decompressor.wg.Done) return decompressor, nil @@ -198,7 +196,7 @@ func newBuiltinDecompressor(engine, extension string, reader io.Reader, logger l return decompressor, err } -// This return a writer that will compress the data using the specified engine before writing to the underlying writer. +// This returns a writer that will compress the data using the specified engine before writing to the underlying writer. func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger) (compressor io.WriteCloser, err error) { switch engine { case "pgzip": @@ -225,7 +223,6 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger compressor = lz4Writer case "zstd": - // compressor = zstd.NewWriterLevel(writer, *compressionLevel) zst, err := zstd.NewWriter(writer, zstd.WithEncoderLevel(zstd.EncoderLevel(*compressionLevel))) if err != nil { return compressor, vterrors.Wrap(err, "cannot create zstd compressor") From 9f61d73c01930f6ba5ab6b8e26dc7b8972a30ab6 Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Thu, 10 Jun 2021 06:56:41 -0700 Subject: [PATCH 05/21] add copyright notices Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/compression.go | 16 ++++++++++++++++ go/vt/mysqlctl/compression_test.go | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index a6e03dfced7..b58af76ba0e 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package mysqlctl import ( diff --git a/go/vt/mysqlctl/compression_test.go b/go/vt/mysqlctl/compression_test.go index 56d8577c97b..386ddb7ab78 100644 --- a/go/vt/mysqlctl/compression_test.go +++ b/go/vt/mysqlctl/compression_test.go @@ -1,3 +1,19 @@ +/* +Copyright 2021 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package mysqlctl import ( From a210501d815662dcbfa29ee93f8b5ff119063632 Mon Sep 17 00:00:00 2001 From: Renan Rangel Date: Thu, 24 Mar 2022 10:04:36 -0700 Subject: [PATCH 06/21] add support for builtin engine Signed-off-by: Renan Rangel Signed-off-by: Rameez Sajwani --- go.mod | 1 + go.sum | 2 + go/vt/mysqlctl/builtinbackupengine.go | 61 +++++++++++++++++---------- go/vt/mysqlctl/compression.go | 28 +++++++++--- go/vt/mysqlctl/compression_test.go | 2 +- go/vt/mysqlctl/xtrabackupengine.go | 9 +--- 6 files changed, 65 insertions(+), 38 deletions(-) diff --git a/go.mod b/go.mod index c3cbbc0189d..e354c7c5c1a 100644 --- a/go.mod +++ b/go.mod @@ -167,6 +167,7 @@ require ( github.com/onsi/ginkgo v1.12.1 // indirect github.com/onsi/gomega v1.10.3 // indirect github.com/pelletier/go-toml v1.9.3 // indirect + github.com/pierrec/lz4 v2.6.1+incompatible // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.2.0 // indirect github.com/prometheus/procfs v0.6.0 // indirect diff --git a/go.sum b/go.sum index d3a99ff84d2..c459800bb2b 100644 --- a/go.sum +++ b/go.sum @@ -330,6 +330,8 @@ github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaS github.com/golang/protobuf v1.5.1/go.mod h1:DopwsBzvsk0Fs44TXzsVbJyPhcCPeIwnvohx4u74HPM= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= +github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.3 h1:fHPg5GQYlCeLIPB9BZqMVR5nR9A+IM5zcgeTdjMYmLA= github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 6fbadf3fea4..7059e627f0b 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -32,9 +32,6 @@ import ( "sync/atomic" "time" - "github.com/klauspost/pgzip" - "github.com/planetscale/pargzip" - "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/sync2" "vitess.io/vitess/go/vt/concurrency" @@ -78,6 +75,9 @@ type builtinBackupManifest struct { // BackupManifest is an anonymous embedding of the base manifest struct. BackupManifest + // CompressionEngine stores which compression engine was used to originally compress the files. + CompressionEngine string `json:",omitempty"` + // FileEntries contains all the files in the backup FileEntries []FileEntry @@ -351,9 +351,10 @@ func (be *BuiltinBackupEngine) backupFiles(ctx context.Context, params BackupPar }, // Builtin-specific fields - FileEntries: fes, - TransformHook: *backupStorageHook, - SkipCompress: !*backupStorageCompress, + FileEntries: fes, + TransformHook: *backupStorageHook, + SkipCompress: !*backupStorageCompress, + CompressionEngine: *builtinCompressor, } data, err := json.MarshalIndent(bm, "", " ") if err != nil { @@ -498,13 +499,19 @@ func (be *BuiltinBackupEngine) backupFile(ctx context.Context, params BackupPara } // Create the gzip compression pipe, if necessary. - var gzip *pargzip.Writer + var compressor io.WriteCloser if *backupStorageCompress { - gzip = pargzip.NewWriter(writer) - gzip.ChunkSize = *backupCompressBlockSize - gzip.Parallel = *backupCompressBlocks - gzip.CompressionLevel = pargzip.BestSpeed - writer = gzip + + if *externalCompressorCmd != "" { + compressor, err = newExternalCompressor(ctx, *externalCompressorCmd, writer, params.Logger) + } else { + compressor, err = newBuiltinCompressor(*builtinCompressor, writer, params.Logger) + } + if err != nil { + return vterrors.Wrap(err, "can't create compressor") + } + + writer = compressor } // Copy from the source file to writer (optional gzip, @@ -515,9 +522,9 @@ func (be *BuiltinBackupEngine) backupFile(ctx context.Context, params BackupPara } // Close gzip to flush it, after that all data is sent to writer. - if gzip != nil { - if err = gzip.Close(); err != nil { - return vterrors.Wrap(err, "cannot close gzip") + if compressor != nil { + if err = compressor.Close(); err != nil { + return vterrors.Wrap(err, "cannot close compressor") } } @@ -599,7 +606,7 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP // And restore the file. name := fmt.Sprintf("%v", i) params.Logger.Infof("Copying file %v: %v", name, fes[i].Name) - err := be.restoreFile(ctx, params, bh, &fes[i], bm.TransformHook, !bm.SkipCompress, name) + err := be.restoreFile(ctx, params, bh, &fes[i], bm.TransformHook, !bm.SkipCompress, bm.CompressionEngine, name) if err != nil { rec.RecordError(vterrors.Wrapf(err, "can't restore file %v to %v", name, fes[i].Name)) } @@ -610,7 +617,7 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP } // restoreFile restores an individual file. -func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle, fe *FileEntry, transformHook string, compress bool, name string) (finalErr error) { +func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle, fe *FileEntry, transformHook string, compress bool, compressionEngine string, name string) (finalErr error) { // Open the source file for reading. source, err := bh.ReadFile(ctx, name) if err != nil { @@ -653,21 +660,29 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa // Create the uncompresser if needed. if compress { - gz, err := pgzip.NewReader(reader) + var decompressor io.ReadCloser + + if *externalDecompressorCmd != "" { + decompressor, err = newExternalDecompressor(ctx, *externalDecompressorCmd, reader, params.Logger) + } else { + decompressor, err = newBuiltinDecompressor(compressionEngine, reader, params.Logger) + } if err != nil { - return vterrors.Wrap(err, "can't open gzip decompressor") + return vterrors.Wrap(err, "can't create decompressor") } + defer func() { - if cerr := gz.Close(); cerr != nil { + if cerr := decompressor.Close(); cerr != nil { + params.Logger.Errorf("failed to close decompressor: %v", cerr) if finalErr != nil { // We already have an error, just log this one. - log.Errorf("failed to close gzip decompressor %v: %v", name, cerr) + log.Errorf("failed to close decompressor %v: %v", name, cerr) } else { - finalErr = vterrors.Wrap(err, "failed to close gzip decompressor") + finalErr = vterrors.Wrap(cerr, "failed to close decompressor") } } }() - reader = gz + reader = decompressor } // Copy the data. Will also write to the hasher. diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index b58af76ba0e..eda0143c6eb 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -38,6 +38,13 @@ import ( var ( compressionLevel = flag.Int("compression_level", 1, "What level to pass to the compressor") + // switch which compressor/decompressor to use + builtinCompressor = flag.String("builtin_compressor", "pgzip", "which builtin compressor engine to use") + builtinDecompressor = flag.String("builtin_decompressor", "auto", "which builtin decompressor engine to use") + // use and external command to decompress the backups + externalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when decompressing a backup") + externalCompressorExt = flag.String("external_compressor_extension", "", "which extension to use when using an external decompressor") + externalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when compressing a backup") errUnsupportedCompressionEngine = errors.New("unsupported engine") errUnsupportedCompressionExtension = errors.New("unsupported extension") @@ -169,9 +176,9 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade return decompressor, nil } -// This returns a reader that will decompress the underlying provided reader and will use the specified supported engine (or -// try to detect which one to use based on the extension if the default "auto" is used. -func newBuiltinDecompressor(engine, extension string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { +// This is a wrapper to get the right decompressor (see below) based on the extension of the file. +func newBuiltinDecompressorFromExtension(extension, engine string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { + // we only infer the engine from the extension is set to "auto", otherwise we use whatever the user selected if engine == "auto" { logger.Infof("Builtin decompressor set to auto, checking which engine to decompress based on the extension") @@ -183,16 +190,25 @@ func newBuiltinDecompressor(engine, extension string, reader io.Reader, logger l engine = eng } + return newBuiltinDecompressor(engine, reader, logger) +} + +// 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 == "pargzip" { + logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") + + engine = "pgzip" + } + switch engine { case "pgzip": d, err := pgzip.NewReader(reader) if err != nil { return nil, err } + decompressor = d - case "pargzip": - err = errors.New("engine pargzip does not support decompression") - return decompressor, err case "lz4": decompressor = ioutil.NopCloser(lz4.NewReader(reader)) case "zstd": diff --git a/go/vt/mysqlctl/compression_test.go b/go/vt/mysqlctl/compression_test.go index 386ddb7ab78..a9137be79ae 100644 --- a/go/vt/mysqlctl/compression_test.go +++ b/go/vt/mysqlctl/compression_test.go @@ -81,7 +81,7 @@ func TestBuiltinCompressors(t *testing.T) { } compressor.Close() - decompressor, err := newBuiltinDecompressor(engine, "not used", &compressed, logger) + decompressor, err := newBuiltinDecompressor(engine, &compressed, logger) if err != nil { t.Error(err) return diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index b248d2297cc..faab6421631 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -61,13 +61,6 @@ var ( // striping mode xtrabackupStripes = flag.Uint("xtrabackup_stripes", 0, "If greater than 0, use data striping across this many destination files to parallelize data transfer and decompression") xtrabackupStripeBlockSize = flag.Uint("xtrabackup_stripe_block_size", 102400, "Size in bytes of each block that gets sent to a given stripe before rotating to the next stripe") - // switch which compressor/decompressor to use - builtinCompressor = flag.String("xtrabackup_builtin_compressor", "pgzip", "which builtin compressor engine to use") - builtinDecompressor = flag.String("xtrabackup_builtin_decompressor", "auto", "which builtin decompressor engine to use") - // use and external command to decompress the backups - externalCompressorCmd = flag.String("xtrabackup_external_compressor", "", "command with arguments to use when decompressing a backup") - externalCompressorExt = flag.String("xtrabackup_external_compressor_extension", "", "which extension to use when using an external decompressor") - externalDecompressorCmd = flag.String("xtrabackup_external_decompressor", "", "command with arguments to use when compressing a backup") ) const ( @@ -565,7 +558,7 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log if *externalDecompressorCmd != "" { decompressor, err = newExternalDecompressor(ctx, *externalDecompressorCmd, reader, logger) } else { - decompressor, err = newBuiltinDecompressor(*builtinDecompressor, extension, reader, logger) + decompressor, err = newBuiltinDecompressorFromExtension(extension, *builtinDecompressor, reader, logger) } if err != nil { return vterrors.Wrap(err, "can't create decompressor") From 8c295f7cf17a481d13cc5d6ae9bfe4f14f3aa4dd Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Tue, 21 Jun 2022 16:03:53 -0700 Subject: [PATCH 07/21] Adding test case for buckup compression Signed-off-by: Rameez Sajwani --- .../backup/mysqlctld/backup_mysqlctld_test.go | 31 ++++++++- .../backup/vtctlbackup/backup_test.go | 31 ++++++++- .../backup/vtctlbackup/backup_utils.go | 56 ++++++++++++++-- .../backup/xtrabackup/xtrabackup_test.go | 43 +++++++++++- .../xtrabackup_stream_test.go | 2 +- go/vt/mysqlctl/builtinbackupengine.go | 12 ++-- go/vt/mysqlctl/compression.go | 10 +-- go/vt/mysqlctl/xtrabackupengine.go | 20 +++--- go/vt/wrangler/testlib/backup_test.go | 65 ++++++++++++++++++- 9 files changed, 236 insertions(+), 34 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index 2ee11103c72..b7c78d446c4 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -24,5 +24,34 @@ import ( // TestBackupMysqlctld - tests the backup using mysqlctld. func TestBackupMysqlctld(t *testing.T) { - backup.TestBackup(t, backup.Mysqlctld, "", 0) + backup.TestBackup(t, backup.Mysqlctld, "", 0, nil) +} + +func TestBackupMainWithlz4Compression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "lz4", + } + + backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) +} + +func TestBackupMainWithPargzipCompression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "pargzip", + } + + backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) +} + +func TestBackupMainWithZstdCompression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + ExternalCompressorCmd: "zstd", + ExternalCompressorExt: ".zst", + ExternalDecompressorCmd: "zstd -d", + } + + backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) } diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go index 6f233eafeda..42cef312480 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_test.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go @@ -22,5 +22,34 @@ import ( // TestBackupMain - main tests backup using vtctl commands func TestBackupMain(t *testing.T) { - TestBackup(t, Backup, "", 0) + TestBackup(t, Backup, "", 0, nil) +} + +func TestBackupMainWithlz4Compression(t *testing.T) { + var cDetails *CompressionDetails + cDetails = &CompressionDetails{ + BuiltinCompressor: "lz4", + } + + TestBackup(t, Backup, "", 0, cDetails) +} + +func TestBackupMainWithPargzipCompression(t *testing.T) { + var cDetails *CompressionDetails + cDetails = &CompressionDetails{ + BuiltinCompressor: "pargzip", + } + + TestBackup(t, Backup, "", 0, cDetails) +} + +func TestBackupMainWithZstdCompression(t *testing.T) { + var cDetails *CompressionDetails + cDetails = &CompressionDetails{ + ExternalCompressorCmd: "zstd", + ExternalCompressorExt: ".zst", + ExternalDecompressorCmd: "zstd -d", + } + + TestBackup(t, Backup, "", 0, cDetails) } diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go index f4e2e92a9ac..497244c60f8 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go @@ -28,6 +28,9 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/test/endtoend/sharding/initialsharding" "vitess.io/vitess/go/vt/mysqlctl" @@ -80,8 +83,16 @@ var ( ) Engine=InnoDB` ) +type CompressionDetails struct { + BuiltinCompressor string + BuiltinDecompressor string + ExternalCompressorCmd string + ExternalCompressorExt string + ExternalDecompressorCmd string +} + // LaunchCluster : starts the cluster as per given params. -func LaunchCluster(setupType int, streamMode string, stripes int) (int, error) { +func LaunchCluster(setupType int, streamMode string, stripes int, cDetails *CompressionDetails) (int, error) { localCluster = cluster.NewCluster(cell, hostname) // Start topo server @@ -136,6 +147,8 @@ func LaunchCluster(setupType int, streamMode string, stripes int) (int, error) { commonTabletArg = append(commonTabletArg, xtrabackupArgs...) } + commonTabletArg = append(commonTabletArg, getCompressorArgs(cDetails)...) + var mysqlProcs []*exec.Cmd for i := 0; i < 3; i++ { tabletType := "replica" @@ -208,13 +221,40 @@ func LaunchCluster(setupType int, streamMode string, stripes int) (int, error) { return 0, nil } +func getCompressorArgs(cDetails *CompressionDetails) []string { + var args []string + + if cDetails == nil { + return args + } + + if cDetails.BuiltinCompressor != "" { + args = append(args, fmt.Sprintf("--builtin_compressor=%s", cDetails.BuiltinCompressor)) + } + if cDetails.BuiltinDecompressor != "" { + args = append(args, fmt.Sprintf("--builtin_decompressor=%s", cDetails.BuiltinDecompressor)) + } + if cDetails.ExternalCompressorCmd != "" { + args = append(args, fmt.Sprintf("--external_compressor=%s", cDetails.ExternalCompressorCmd)) + } + if cDetails.ExternalCompressorExt != "" { + args = append(args, fmt.Sprintf("--external_compressor_extension=%s", cDetails.ExternalCompressorExt)) + } + if cDetails.ExternalDecompressorCmd != "" { + args = append(args, fmt.Sprintf("--external_decompressor=%s", cDetails.ExternalDecompressorCmd)) + } + + return args + +} + // TearDownCluster shuts down all cluster processes func TearDownCluster() { localCluster.Teardown() } // TestBackup runs all the backup tests -func TestBackup(t *testing.T, setupType int, streamMode string, stripes int) { +func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDetails *CompressionDetails) error { testMethods := []struct { name string @@ -235,7 +275,7 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int) { { name: "TestPrimaryBackup", method: primaryBackup, - }, // + }, { name: "TestPrimaryReplicaSameBackup", method: primaryReplicaSameBackup, @@ -257,7 +297,7 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int) { defer cluster.PanicHandler(t) // setup cluster for the testing - code, err := LaunchCluster(setupType, streamMode, stripes) + code, err := LaunchCluster(setupType, streamMode, stripes, cDetails) require.Nilf(t, err, "setup failed with status code %d", code) // Teardown the cluster @@ -266,9 +306,11 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int) { // Run all the backup tests for _, test := range testMethods { - t.Run(test.name, test.method) + if retVal := t.Run(test.name, test.method); retVal == false { + return vterrors.Errorf(vtrpc.Code_UNKNOWN, "test failure: %s", test.name) + } } - + return nil } type restoreMethod func(t *testing.T, tablet *cluster.Vttablet) @@ -301,7 +343,7 @@ func primaryBackup(t *testing.T) { require.Nil(t, err) // We'll restore this on the primary later to test restores using a backup timestamp - firstBackupTimestamp := time.Now().Format(mysqlctl.BackupTimestampFormat) + firstBackupTimestamp := time.Now().UTC().Format(mysqlctl.BackupTimestampFormat) backups := localCluster.VerifyBackupCount(t, shardKsName, 1) assert.Contains(t, backups[0], primary.Alias) diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 7dbef9df25e..007d0643f80 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -19,10 +19,51 @@ package vtctlbackup import ( "testing" + "github.com/stretchr/testify/require" + backup "vitess.io/vitess/go/test/endtoend/backup/vtctlbackup" ) // TestXtraBackup - tests the backup using xtrabackup func TestXtrabackup(t *testing.T) { - backup.TestBackup(t, backup.XtraBackup, "tar", 0) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, nil) +} + +func TestBackupMainWithlz4Compression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "lz4", + } + + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) +} + +func TestBackupMainWithPargzipCompression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "pargzip", + } + + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) +} + +func TestBackupMainWithError(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "pargzip", + BuiltinDecompressor: "lz4", + } + err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) + require.EqualError(t, err, "test failure: TestReplicaBackup") +} + +func TestBackupMainWithZstdCompression(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + ExternalCompressorCmd: "zstd", + ExternalCompressorExt: ".zst", + ExternalDecompressorCmd: "zstd -d", + } + + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) } diff --git a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go index 99eec5a8c53..9fb38dbdd6a 100644 --- a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go +++ b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go @@ -24,5 +24,5 @@ import ( // TestXtrabackupStream - tests the backup using xtrabackup with xbstream mode func TestXtrabackupStream(t *testing.T) { - backup.TestBackup(t, backup.XtraBackup, "xbstream", 8) + backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, nil) } diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 7059e627f0b..8a92e16c19b 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -354,7 +354,7 @@ func (be *BuiltinBackupEngine) backupFiles(ctx context.Context, params BackupPar FileEntries: fes, TransformHook: *backupStorageHook, SkipCompress: !*backupStorageCompress, - CompressionEngine: *builtinCompressor, + CompressionEngine: *BuiltinCompressor, } data, err := json.MarshalIndent(bm, "", " ") if err != nil { @@ -502,10 +502,10 @@ func (be *BuiltinBackupEngine) backupFile(ctx context.Context, params BackupPara var compressor io.WriteCloser if *backupStorageCompress { - if *externalCompressorCmd != "" { - compressor, err = newExternalCompressor(ctx, *externalCompressorCmd, writer, params.Logger) + if *ExternalCompressorCmd != "" { + compressor, err = newExternalCompressor(ctx, *ExternalCompressorCmd, writer, params.Logger) } else { - compressor, err = newBuiltinCompressor(*builtinCompressor, writer, params.Logger) + compressor, err = newBuiltinCompressor(*BuiltinCompressor, writer, params.Logger) } if err != nil { return vterrors.Wrap(err, "can't create compressor") @@ -662,8 +662,8 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa if compress { var decompressor io.ReadCloser - if *externalDecompressorCmd != "" { - decompressor, err = newExternalDecompressor(ctx, *externalDecompressorCmd, reader, params.Logger) + if *ExternalDecompressorCmd != "" { + decompressor, err = newExternalDecompressor(ctx, *ExternalDecompressorCmd, reader, params.Logger) } else { decompressor, err = newBuiltinDecompressor(compressionEngine, reader, params.Logger) } diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index eda0143c6eb..66445c8ad79 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -39,12 +39,12 @@ import ( var ( compressionLevel = flag.Int("compression_level", 1, "What level to pass to the compressor") // switch which compressor/decompressor to use - builtinCompressor = flag.String("builtin_compressor", "pgzip", "which builtin compressor engine to use") - builtinDecompressor = flag.String("builtin_decompressor", "auto", "which builtin decompressor engine to use") + BuiltinCompressor = flag.String("builtin_compressor", "pgzip", "which builtin compressor engine to use") + BuiltinDecompressor = flag.String("builtin_decompressor", "auto", "which builtin decompressor engine to use") // use and external command to decompress the backups - externalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when decompressing a backup") - externalCompressorExt = flag.String("external_compressor_extension", "", "which extension to use when using an external decompressor") - externalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when compressing a backup") + ExternalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when decompressing a backup") + ExternalCompressorExt = flag.String("external_compressor_extension", "", "which extension to use when using an external decompressor") + ExternalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when compressing a backup") errUnsupportedCompressionEngine = errors.New("unsupported engine") errUnsupportedCompressionExtension = errors.New("unsupported extension") diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index faab6421631..9dc0e146d40 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -106,10 +106,10 @@ func (be *XtrabackupEngine) backupFileName() string { fileName += *xtrabackupStreamMode } if *backupStorageCompress { - if *externalDecompressorCmd != "" { - fileName += *externalCompressorExt + if *ExternalDecompressorCmd != "" { + fileName += *ExternalCompressorExt } else { - if ext, err := getExtensionFromEngine(*builtinCompressor); err != nil { + if ext, err := getExtensionFromEngine(*BuiltinCompressor); err != nil { // there is a check for this, but just in case that fails, we set a extension to the file fileName += ".unknown" } else { @@ -139,7 +139,7 @@ func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupPara } // an extension is required when using an external compressor - if *backupStorageCompress && *externalCompressorCmd != "" && *externalCompressorExt == "" { + if *backupStorageCompress && *ExternalCompressorCmd != "" && *ExternalCompressorExt == "" { return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, "xtrabackup_external_compressor_extension not provided when using an external compressor") } @@ -296,10 +296,10 @@ func (be *XtrabackupEngine) backupFiles(ctx context.Context, params BackupParams if *backupStorageCompress { var compressor io.WriteCloser - if *externalCompressorCmd != "" { - compressor, err = newExternalCompressor(ctx, *externalCompressorCmd, writer, params.Logger) + if *ExternalCompressorCmd != "" { + compressor, err = newExternalCompressor(ctx, *ExternalCompressorCmd, writer, params.Logger) } else { - compressor, err = newBuiltinCompressor(*builtinCompressor, writer, params.Logger) + compressor, err = newBuiltinCompressor(*BuiltinCompressor, writer, params.Logger) } if err != nil { return replicationPosition, vterrors.Wrap(err, "can't create compressor") @@ -555,10 +555,10 @@ func (be *XtrabackupEngine) extractFiles(ctx context.Context, logger logutil.Log if compressed { var decompressor io.ReadCloser - if *externalDecompressorCmd != "" { - decompressor, err = newExternalDecompressor(ctx, *externalDecompressorCmd, reader, logger) + if *ExternalDecompressorCmd != "" { + decompressor, err = newExternalDecompressor(ctx, *ExternalDecompressorCmd, reader, logger) } else { - decompressor, err = newBuiltinDecompressorFromExtension(extension, *builtinDecompressor, reader, logger) + decompressor, err = newBuiltinDecompressorFromExtension(extension, *BuiltinDecompressor, reader, logger) } if err != nil { return vterrors.Wrap(err, "can't create decompressor") diff --git a/go/vt/wrangler/testlib/backup_test.go b/go/vt/wrangler/testlib/backup_test.go index e77e2353d98..962b4f9f093 100644 --- a/go/vt/wrangler/testlib/backup_test.go +++ b/go/vt/wrangler/testlib/backup_test.go @@ -24,9 +24,8 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "vitess.io/vitess/go/vt/discovery" @@ -46,7 +45,52 @@ import ( topodatapb "vitess.io/vitess/go/vt/proto/topodata" ) +type compressionDetails struct { + BuiltinCompressor string + BuiltinDecompressor string + ExternalCompressorCmd string + ExternalCompressorExt string + ExternalDecompressorCmd string +} + func TestBackupRestore(t *testing.T) { + testBackupRestore(t, nil) +} + +func TestBackupRestoreWithLz4(t *testing.T) { + var cDetails *compressionDetails + cDetails = &compressionDetails{ + BuiltinCompressor: "lz4", + } + + testBackupRestore(t, cDetails) +} + +// TODO: @rameez. I was expecting this test to fail but it turns out +// we infer decompressor through compression engine in builtinEngine. +// It is only in xtrabackup where we infer decompressor through extension & BuiltinDecompressor param. +func TestBackupRestoreWithCompressionDecompression(t *testing.T) { + var cDetails *compressionDetails + cDetails = &compressionDetails{ + BuiltinCompressor: "lz4", + BuiltinDecompressor: "pargzip", + } + + testBackupRestore(t, cDetails) +} + +func TestBackupRestoreWithExternalCompression(t *testing.T) { + var cDetails *compressionDetails + cDetails = &compressionDetails{ + ExternalCompressorCmd: "gzip", + ExternalCompressorExt: ".gz", + ExternalDecompressorCmd: "gzip -d", + } + + testBackupRestore(t, cDetails) +} + +func testBackupRestore(t *testing.T, cDetails *compressionDetails) { delay := discovery.GetTabletPickerRetryDelay() defer func() { discovery.SetTabletPickerRetryDelay(delay) @@ -82,6 +126,23 @@ func TestBackupRestore(t *testing.T) { fbsRoot := path.Join(root, "fbs") *filebackupstorage.FileBackupStorageRoot = fbsRoot *backupstorage.BackupStorageImplementation = "file" + if cDetails != nil { + if cDetails.BuiltinCompressor != "" { + *mysqlctl.BuiltinCompressor = cDetails.BuiltinCompressor + } + if cDetails.BuiltinDecompressor != "" { + *mysqlctl.BuiltinDecompressor = cDetails.BuiltinDecompressor + } + if cDetails.ExternalCompressorCmd != "" { + *mysqlctl.ExternalCompressorCmd = cDetails.ExternalCompressorCmd + } + if cDetails.ExternalCompressorExt != "" { + *mysqlctl.ExternalCompressorExt = cDetails.ExternalCompressorExt + } + if cDetails.ExternalDecompressorCmd != "" { + *mysqlctl.ExternalDecompressorCmd = cDetails.ExternalDecompressorCmd + } + } // Initialize the fake mysql root directories sourceInnodbDataDir := path.Join(root, "source_innodb_data") From a1556894e8440aa320c0ec446433761f0229f9df Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Wed, 22 Jun 2022 22:42:22 -0700 Subject: [PATCH 08/21] Fixing unit test and run mod tidy Signed-off-by: Rameez Sajwani --- go.mod | 1 - go.sum | 2 -- go/flags/endtoend/vttablet.txt | 12 ++++++++++++ go/vt/mysqlctl/compression.go | 8 ++++---- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index e354c7c5c1a..c3cbbc0189d 100644 --- a/go.mod +++ b/go.mod @@ -167,7 +167,6 @@ require ( github.com/onsi/ginkgo v1.12.1 // indirect github.com/onsi/gomega v1.10.3 // indirect github.com/pelletier/go-toml v1.9.3 // indirect - github.com/pierrec/lz4 v2.6.1+incompatible // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/client_model v0.2.0 // indirect github.com/prometheus/procfs v0.6.0 // indirect diff --git a/go.sum b/go.sum index c459800bb2b..d3a99ff84d2 100644 --- a/go.sum +++ b/go.sum @@ -330,8 +330,6 @@ github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaS github.com/golang/protobuf v1.5.1/go.mod h1:DopwsBzvsk0Fs44TXzsVbJyPhcCPeIwnvohx4u74HPM= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/golang/snappy v0.0.1 h1:Qgr9rKW7uDUkrbSmQeiDsGa8SjGyCOGtuasMWwvp2P4= -github.com/golang/snappy v0.0.1/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/golang/snappy v0.0.3 h1:fHPg5GQYlCeLIPB9BZqMVR5nR9A+IM5zcgeTdjMYmLA= github.com/golang/snappy v0.0.3/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEWrmP2Q= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index 5816ca1bb6d..d1c6ba7e6c8 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/go/flags/endtoend/vttablet.txt @@ -59,6 +59,10 @@ Usage of vttablet: (DEPRECATED) True if and only if the binlog streamer should use V3-style sharding, which doesn't require a preset sharding key column. (default true) --binlog_user string PITR restore parameter: username of binlog server. + --builtin_compressor string + builtin compressor engine to use (default pgzip) + --builtin_decompressor string + builtin decompressor engine to use (default auto) --builtinbackup_mysqld_timeout duration how long to wait for mysqld to shutdown at the start of the backup (default 10m0s) --builtinbackup_progress duration @@ -69,6 +73,8 @@ Usage of vttablet: Path to JSON config file for ceph backup storage (default ceph_backup_config.json) --client-found-rows-pool-size int DEPRECATED: queryserver-config-transaction-cap will be used instead. + --compression_level int + what level to pass to the compressor (default 1) --consul_auth_static_file string JSON File to read the topos/tokens from. --cpu_profile string @@ -407,6 +413,12 @@ Usage of vttablet: if this flag is true, vttablet will fail to start if a valid tableacl config does not exist --enforce_strict_trans_tables If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES or STRICT_ALL_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database. (default true) + --external_compressor string + command with arguments to use when decompressing a backup + --external_compressor_extension string + extension to use when using an external decompressor + --external_decompressor string + command with arguments to use when compressing a backup --file_backup_storage_root string root directory for the file backup storage --filecustomrules string diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index 66445c8ad79..d985e9f920e 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -37,13 +37,13 @@ import ( ) var ( - compressionLevel = flag.Int("compression_level", 1, "What level to pass to the compressor") + compressionLevel = flag.Int("compression_level", 1, "what level to pass to the compressor") // switch which compressor/decompressor to use - BuiltinCompressor = flag.String("builtin_compressor", "pgzip", "which builtin compressor engine to use") - BuiltinDecompressor = flag.String("builtin_decompressor", "auto", "which builtin decompressor engine to use") + BuiltinCompressor = flag.String("builtin_compressor", "pgzip", "builtin compressor engine to use") + BuiltinDecompressor = flag.String("builtin_decompressor", "auto", "builtin decompressor engine to use") // use and external command to decompress the backups ExternalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when decompressing a backup") - ExternalCompressorExt = flag.String("external_compressor_extension", "", "which extension to use when using an external decompressor") + ExternalCompressorExt = flag.String("external_compressor_extension", "", "extension to use when using an external decompressor") ExternalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when compressing a backup") errUnsupportedCompressionEngine = errors.New("unsupported engine") From e6a84566ea98d1e74e1c17912a911ed2437b9a87 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Sat, 25 Jun 2022 00:11:37 -0700 Subject: [PATCH 09/21] Removing unwanted unit tests Signed-off-by: Rameez Sajwani --- .../backup/mysqlctld/backup_mysqlctld_test.go | 20 -------------- .../backup/vtctlbackup/backup_test.go | 18 ------------- .../backup/xtrabackup/xtrabackup_test.go | 9 ------- go/vt/wrangler/testlib/backup_test.go | 26 +++---------------- 4 files changed, 3 insertions(+), 70 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index b7c78d446c4..4a893595f5f 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -35,23 +35,3 @@ func TestBackupMainWithlz4Compression(t *testing.T) { backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) } - -func TestBackupMainWithPargzipCompression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ - BuiltinCompressor: "pargzip", - } - - backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) -} - -func TestBackupMainWithZstdCompression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ - ExternalCompressorCmd: "zstd", - ExternalCompressorExt: ".zst", - ExternalDecompressorCmd: "zstd -d", - } - - backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) -} diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go index 42cef312480..f7e973fa7c2 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_test.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go @@ -25,24 +25,6 @@ func TestBackupMain(t *testing.T) { TestBackup(t, Backup, "", 0, nil) } -func TestBackupMainWithlz4Compression(t *testing.T) { - var cDetails *CompressionDetails - cDetails = &CompressionDetails{ - BuiltinCompressor: "lz4", - } - - TestBackup(t, Backup, "", 0, cDetails) -} - -func TestBackupMainWithPargzipCompression(t *testing.T) { - var cDetails *CompressionDetails - cDetails = &CompressionDetails{ - BuiltinCompressor: "pargzip", - } - - TestBackup(t, Backup, "", 0, cDetails) -} - func TestBackupMainWithZstdCompression(t *testing.T) { var cDetails *CompressionDetails cDetails = &CompressionDetails{ diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 007d0643f80..e02ba6a0d0a 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -38,15 +38,6 @@ func TestBackupMainWithlz4Compression(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) } -func TestBackupMainWithPargzipCompression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ - BuiltinCompressor: "pargzip", - } - - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) -} - func TestBackupMainWithError(t *testing.T) { var cDetails *backup.CompressionDetails cDetails = &backup.CompressionDetails{ diff --git a/go/vt/wrangler/testlib/backup_test.go b/go/vt/wrangler/testlib/backup_test.go index 962b4f9f093..a52bebd20a3 100644 --- a/go/vt/wrangler/testlib/backup_test.go +++ b/go/vt/wrangler/testlib/backup_test.go @@ -57,34 +57,14 @@ func TestBackupRestore(t *testing.T) { testBackupRestore(t, nil) } -func TestBackupRestoreWithLz4(t *testing.T) { - var cDetails *compressionDetails - cDetails = &compressionDetails{ - BuiltinCompressor: "lz4", - } - - testBackupRestore(t, cDetails) -} - // TODO: @rameez. I was expecting this test to fail but it turns out // we infer decompressor through compression engine in builtinEngine. // It is only in xtrabackup where we infer decompressor through extension & BuiltinDecompressor param. -func TestBackupRestoreWithCompressionDecompression(t *testing.T) { - var cDetails *compressionDetails - cDetails = &compressionDetails{ - BuiltinCompressor: "lz4", - BuiltinDecompressor: "pargzip", - } - - testBackupRestore(t, cDetails) -} - -func TestBackupRestoreWithExternalCompression(t *testing.T) { +func TestBackupRestoreWithPargzip(t *testing.T) { var cDetails *compressionDetails cDetails = &compressionDetails{ - ExternalCompressorCmd: "gzip", - ExternalCompressorExt: ".gz", - ExternalDecompressorCmd: "gzip -d", + BuiltinCompressor: "pargzip", + BuiltinDecompressor: "lz4", } testBackupRestore(t, cDetails) From 38186ad11442854ae69fa4c08956f3008f1fb65d Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Sat, 25 Jun 2022 02:07:32 -0700 Subject: [PATCH 10/21] Increase timeout of backup tests Signed-off-by: Rameez Sajwani --- .../backup/xtrabackup/xtrabackup_test.go | 20 +++++++++---------- test/config.json | 6 +++--- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index e02ba6a0d0a..68f29351670 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -38,16 +38,6 @@ func TestBackupMainWithlz4Compression(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) } -func TestBackupMainWithError(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ - BuiltinCompressor: "pargzip", - BuiltinDecompressor: "lz4", - } - err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) - require.EqualError(t, err, "test failure: TestReplicaBackup") -} - func TestBackupMainWithZstdCompression(t *testing.T) { var cDetails *backup.CompressionDetails cDetails = &backup.CompressionDetails{ @@ -58,3 +48,13 @@ func TestBackupMainWithZstdCompression(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) } + +func TestBackupMainWithError(t *testing.T) { + var cDetails *backup.CompressionDetails + cDetails = &backup.CompressionDetails{ + BuiltinCompressor: "pargzip", + BuiltinDecompressor: "lz4", + } + err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) + require.EqualError(t, err, "test failure: TestReplicaBackup") +} diff --git a/test/config.json b/test/config.json index 5892f63d138..ab3d82a0393 100644 --- a/test/config.json +++ b/test/config.json @@ -104,7 +104,7 @@ }, "backup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/vtctlbackup"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/vtctlbackup", "-timeout", "20m"], "Command": [], "Manual": false, "Shard": "vtctlbackup_sharded_clustertest_heavy", @@ -113,7 +113,7 @@ }, "backup_mysqlctld": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/mysqlctld"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/mysqlctld", "-timeout", "20m"], "Command": [], "Manual": false, "Shard": "21", @@ -149,7 +149,7 @@ }, "backup_xtrabackup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "30m"], "Command": [], "Manual": false, "Shard": "xb_backup", From 95afc73f1ef41fcb06ddd54372467b8afddd29c0 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Sat, 25 Jun 2022 12:32:48 -0700 Subject: [PATCH 11/21] fixing linter errors Signed-off-by: Rameez Sajwani --- .../endtoend/backup/mysqlctld/backup_mysqlctld_test.go | 3 +-- go/test/endtoend/backup/vtctlbackup/backup_utils.go | 2 +- go/test/endtoend/backup/xtrabackup/xtrabackup_test.go | 9 +++------ go/vt/wrangler/testlib/backup_test.go | 3 +-- test/config.json | 2 +- 5 files changed, 7 insertions(+), 12 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index 4a893595f5f..de47b8f1481 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -28,8 +28,7 @@ func TestBackupMysqlctld(t *testing.T) { } func TestBackupMainWithlz4Compression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ + cDetails := &backup.CompressionDetails{ BuiltinCompressor: "lz4", } diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go index 497244c60f8..79d9257bca6 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go @@ -306,7 +306,7 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDe // Run all the backup tests for _, test := range testMethods { - if retVal := t.Run(test.name, test.method); retVal == false { + if retVal := t.Run(test.name, test.method); !retVal { return vterrors.Errorf(vtrpc.Code_UNKNOWN, "test failure: %s", test.name) } } diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 68f29351670..72a7213a4c8 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -30,8 +30,7 @@ func TestXtrabackup(t *testing.T) { } func TestBackupMainWithlz4Compression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ + cDetails := &backup.CompressionDetails{ BuiltinCompressor: "lz4", } @@ -39,8 +38,7 @@ func TestBackupMainWithlz4Compression(t *testing.T) { } func TestBackupMainWithZstdCompression(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ + cDetails := &backup.CompressionDetails{ ExternalCompressorCmd: "zstd", ExternalCompressorExt: ".zst", ExternalDecompressorCmd: "zstd -d", @@ -50,8 +48,7 @@ func TestBackupMainWithZstdCompression(t *testing.T) { } func TestBackupMainWithError(t *testing.T) { - var cDetails *backup.CompressionDetails - cDetails = &backup.CompressionDetails{ + cDetails := &backup.CompressionDetails{ BuiltinCompressor: "pargzip", BuiltinDecompressor: "lz4", } diff --git a/go/vt/wrangler/testlib/backup_test.go b/go/vt/wrangler/testlib/backup_test.go index a52bebd20a3..c3f1c43a083 100644 --- a/go/vt/wrangler/testlib/backup_test.go +++ b/go/vt/wrangler/testlib/backup_test.go @@ -61,8 +61,7 @@ func TestBackupRestore(t *testing.T) { // we infer decompressor through compression engine in builtinEngine. // It is only in xtrabackup where we infer decompressor through extension & BuiltinDecompressor param. func TestBackupRestoreWithPargzip(t *testing.T) { - var cDetails *compressionDetails - cDetails = &compressionDetails{ + cDetails := &compressionDetails{ BuiltinCompressor: "pargzip", BuiltinDecompressor: "lz4", } diff --git a/test/config.json b/test/config.json index ab3d82a0393..68298cdd082 100644 --- a/test/config.json +++ b/test/config.json @@ -149,7 +149,7 @@ }, "backup_xtrabackup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "30m"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "40m"], "Command": [], "Manual": false, "Shard": "xb_backup", From 2f72baa780b4ab9230a03cc74395c5af7c30613c Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Sun, 26 Jun 2022 17:00:55 -0700 Subject: [PATCH 12/21] Change test logic to accomodate running selective tests Signed-off-by: Rameez Sajwani --- .../backup/mysqlctld/backup_mysqlctld_test.go | 4 ++-- .../endtoend/backup/vtctlbackup/backup_test.go | 4 ++-- .../endtoend/backup/vtctlbackup/backup_utils.go | 15 +++++++++++++-- .../endtoend/backup/xtrabackup/xtrabackup_test.go | 9 +++++---- .../xtrabackupstream/xtrabackup_stream_test.go | 2 +- test/config.json | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index de47b8f1481..874f2c598dd 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -24,7 +24,7 @@ import ( // TestBackupMysqlctld - tests the backup using mysqlctld. func TestBackupMysqlctld(t *testing.T) { - backup.TestBackup(t, backup.Mysqlctld, "", 0, nil) + backup.TestBackup(t, backup.Mysqlctld, "", 0, nil, nil) } func TestBackupMainWithlz4Compression(t *testing.T) { @@ -32,5 +32,5 @@ func TestBackupMainWithlz4Compression(t *testing.T) { BuiltinCompressor: "lz4", } - backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails) + backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go index f7e973fa7c2..42ed6edbc38 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_test.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go @@ -22,7 +22,7 @@ import ( // TestBackupMain - main tests backup using vtctl commands func TestBackupMain(t *testing.T) { - TestBackup(t, Backup, "", 0, nil) + TestBackup(t, Backup, "", 0, nil, nil) } func TestBackupMainWithZstdCompression(t *testing.T) { @@ -33,5 +33,5 @@ func TestBackupMainWithZstdCompression(t *testing.T) { ExternalDecompressorCmd: "zstd -d", } - TestBackup(t, Backup, "", 0, cDetails) + TestBackup(t, Backup, "", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go index 79d9257bca6..3d81cc40a88 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go @@ -254,7 +254,7 @@ func TearDownCluster() { } // TestBackup runs all the backup tests -func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDetails *CompressionDetails) error { +func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDetails *CompressionDetails, runSpecific []string) error { testMethods := []struct { name string @@ -295,7 +295,6 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDe } defer cluster.PanicHandler(t) - // setup cluster for the testing code, err := LaunchCluster(setupType, streamMode, stripes, cDetails) require.Nilf(t, err, "setup failed with status code %d", code) @@ -306,6 +305,9 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDe // Run all the backup tests for _, test := range testMethods { + if len(runSpecific) > 0 && !isRegistered(test.name, runSpecific) { + continue + } if retVal := t.Run(test.name, test.method); !retVal { return vterrors.Errorf(vtrpc.Code_UNKNOWN, "test failure: %s", test.name) } @@ -313,6 +315,15 @@ func TestBackup(t *testing.T, setupType int, streamMode string, stripes int, cDe return nil } +func isRegistered(name string, runlist []string) bool { + for _, f := range runlist { + if f == name { + return true + } + } + return false +} + type restoreMethod func(t *testing.T, tablet *cluster.Vttablet) // 1. create a shard with primary and replica1 only diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 72a7213a4c8..89ba7d4c0d9 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -26,7 +26,7 @@ import ( // TestXtraBackup - tests the backup using xtrabackup func TestXtrabackup(t *testing.T) { - backup.TestBackup(t, backup.XtraBackup, "tar", 0, nil) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, nil, nil) } func TestBackupMainWithlz4Compression(t *testing.T) { @@ -34,7 +34,7 @@ func TestBackupMainWithlz4Compression(t *testing.T) { BuiltinCompressor: "lz4", } - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } func TestBackupMainWithZstdCompression(t *testing.T) { @@ -44,7 +44,7 @@ func TestBackupMainWithZstdCompression(t *testing.T) { ExternalDecompressorCmd: "zstd -d", } - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } func TestBackupMainWithError(t *testing.T) { @@ -52,6 +52,7 @@ func TestBackupMainWithError(t *testing.T) { BuiltinCompressor: "pargzip", BuiltinDecompressor: "lz4", } - err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails) + + err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) require.EqualError(t, err, "test failure: TestReplicaBackup") } diff --git a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go index 9fb38dbdd6a..9f76d166992 100644 --- a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go +++ b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go @@ -24,5 +24,5 @@ import ( // TestXtrabackupStream - tests the backup using xtrabackup with xbstream mode func TestXtrabackupStream(t *testing.T) { - backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, nil) + backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, nil, nil) } diff --git a/test/config.json b/test/config.json index 68298cdd082..ab3d82a0393 100644 --- a/test/config.json +++ b/test/config.json @@ -149,7 +149,7 @@ }, "backup_xtrabackup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "40m"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "30m"], "Command": [], "Manual": false, "Shard": "xb_backup", From 9f7812aa7a4a14a7ef632781d6d08c8b465f87ee Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Sun, 26 Jun 2022 21:41:12 -0700 Subject: [PATCH 13/21] removing lint warning Signed-off-by: Rameez Sajwani --- go/test/endtoend/backup/vtctlbackup/backup_test.go | 3 +-- go/test/endtoend/backup/xtrabackup/xtrabackup_test.go | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go index 42ed6edbc38..6c76bfba720 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_test.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go @@ -26,8 +26,7 @@ func TestBackupMain(t *testing.T) { } func TestBackupMainWithZstdCompression(t *testing.T) { - var cDetails *CompressionDetails - cDetails = &CompressionDetails{ + cDetails := &CompressionDetails{ ExternalCompressorCmd: "zstd", ExternalCompressorExt: ".zst", ExternalDecompressorCmd: "zstd -d", diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 89ba7d4c0d9..a2ad784eecd 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -34,7 +34,7 @@ func TestBackupMainWithlz4Compression(t *testing.T) { BuiltinCompressor: "lz4", } - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup"}) } func TestBackupMainWithZstdCompression(t *testing.T) { @@ -44,7 +44,7 @@ func TestBackupMainWithZstdCompression(t *testing.T) { ExternalDecompressorCmd: "zstd -d", } - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) + backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup"}) } func TestBackupMainWithError(t *testing.T) { @@ -53,6 +53,6 @@ func TestBackupMainWithError(t *testing.T) { BuiltinDecompressor: "lz4", } - err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) + err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestPrimaryBackup"}) require.EqualError(t, err, "test failure: TestReplicaBackup") } From 46fde1d89409a3ef0030f3b33af9150e39601e45 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Mon, 27 Jun 2022 07:36:16 -0700 Subject: [PATCH 14/21] fixing test failure Signed-off-by: Rameez Sajwani --- go/test/endtoend/backup/xtrabackup/xtrabackup_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index a2ad784eecd..c3c417b69aa 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -54,5 +54,5 @@ func TestBackupMainWithError(t *testing.T) { } err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestPrimaryBackup"}) - require.EqualError(t, err, "test failure: TestReplicaBackup") + require.EqualError(t, err, "test failure: TestPrimaryBackup") } From a2d96faa5957308d38c706547c760c253f323f34 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Mon, 27 Jun 2022 14:32:32 -0700 Subject: [PATCH 15/21] Removing un-necessary test Signed-off-by: Rameez Sajwani --- .../endtoend/backup/xtrabackup/xtrabackup_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index c3c417b69aa..955cef28682 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -19,8 +19,6 @@ package vtctlbackup import ( "testing" - "github.com/stretchr/testify/require" - backup "vitess.io/vitess/go/test/endtoend/backup/vtctlbackup" ) @@ -46,13 +44,3 @@ func TestBackupMainWithZstdCompression(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup"}) } - -func TestBackupMainWithError(t *testing.T) { - cDetails := &backup.CompressionDetails{ - BuiltinCompressor: "pargzip", - BuiltinDecompressor: "lz4", - } - - err := backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestPrimaryBackup"}) - require.EqualError(t, err, "test failure: TestPrimaryBackup") -} From 13f5e071a1f818181fd13a168d97efb416e5c519 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Tue, 28 Jun 2022 10:33:35 -0700 Subject: [PATCH 16/21] Fixing code review feeback Signed-off-by: Rameez Sajwani --- .../endtoend/backup/mysqlctld/backup_mysqlctld_test.go | 2 +- go/test/endtoend/backup/xtrabackup/xtrabackup_test.go | 10 +--------- .../backup/xtrabackupstream/xtrabackup_stream_test.go | 8 ++++++++ go/vt/mysqlctl/compression.go | 7 ++----- test/config.json | 6 +++--- 5 files changed, 15 insertions(+), 18 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index 874f2c598dd..437435dfeb9 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -27,7 +27,7 @@ func TestBackupMysqlctld(t *testing.T) { backup.TestBackup(t, backup.Mysqlctld, "", 0, nil, nil) } -func TestBackupMainWithlz4Compression(t *testing.T) { +func TestBackupMysqlctldWithlz4Compression(t *testing.T) { cDetails := &backup.CompressionDetails{ BuiltinCompressor: "lz4", } diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 955cef28682..03c60aafb9e 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -27,15 +27,7 @@ func TestXtrabackup(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, nil, nil) } -func TestBackupMainWithlz4Compression(t *testing.T) { - cDetails := &backup.CompressionDetails{ - BuiltinCompressor: "lz4", - } - - backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup"}) -} - -func TestBackupMainWithZstdCompression(t *testing.T) { +func TestXtrabackWithZstdCompression(t *testing.T) { cDetails := &backup.CompressionDetails{ ExternalCompressorCmd: "zstd", ExternalCompressorExt: ".zst", diff --git a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go index 9f76d166992..89815923740 100644 --- a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go +++ b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go @@ -26,3 +26,11 @@ import ( func TestXtrabackupStream(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, nil, nil) } + +func TestXtrabackupStreamWithlz4Compression(t *testing.T) { + cDetails := &backup.CompressionDetails{ + BuiltinCompressor: "lz4", + } + + backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, cDetails, []string{"TestReplicaBackup"}) +} diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index d985e9f920e..a12e73d86f2 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -223,7 +223,7 @@ func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logg return decompressor, err } - logger.Infof("Decompressing backup using built-in engine %q", engine) + logger.Infof("Decompressing backup using engine %q", engine) return decompressor, err } @@ -238,21 +238,18 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger } gzip.SetConcurrency(*backupCompressBlockSize, *backupCompressBlocks) - compressor = gzip case "pargzip": gzip := pargzip.NewWriter(writer) gzip.ChunkSize = *backupCompressBlockSize gzip.Parallel = *backupCompressBlocks gzip.CompressionLevel = *compressionLevel - compressor = gzip case "lz4": lz4Writer := lz4.NewWriter(writer).WithConcurrency(*backupCompressBlocks) lz4Writer.Header = lz4.Header{ CompressionLevel: *compressionLevel, } - compressor = lz4Writer case "zstd": zst, err := zstd.NewWriter(writer, zstd.WithEncoderLevel(zstd.EncoderLevel(*compressionLevel))) @@ -266,7 +263,7 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger return compressor, err } - logger.Infof("Compressing backup using built-in engine %q", engine) + logger.Infof("Compressing backup using engine %q", engine) return } diff --git a/test/config.json b/test/config.json index ab3d82a0393..53c2ad438c6 100644 --- a/test/config.json +++ b/test/config.json @@ -104,7 +104,7 @@ }, "backup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/vtctlbackup", "-timeout", "20m"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/vtctlbackup", "-timeout", "15m"], "Command": [], "Manual": false, "Shard": "vtctlbackup_sharded_clustertest_heavy", @@ -113,7 +113,7 @@ }, "backup_mysqlctld": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/mysqlctld", "-timeout", "20m"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/mysqlctld", "-timeout", "15m"], "Command": [], "Manual": false, "Shard": "21", @@ -149,7 +149,7 @@ }, "backup_xtrabackup": { "File": "unused.go", - "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup", "-timeout", "30m"], + "Args": ["vitess.io/vitess/go/test/endtoend/backup/xtrabackup"], "Command": [], "Manual": false, "Shard": "xb_backup", From 8d762762628bc02ab627825883c271ce9ba5a1a3 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Thu, 30 Jun 2022 15:29:20 -0700 Subject: [PATCH 17/21] Change builtinEngine to consider 'auto' decompressor Signed-off-by: Rameez Sajwani --- .../backup/mysqlctld/backup_mysqlctld_test.go | 11 ++++++++ .../backup/vtctlbackup/backup_test.go | 11 ++++++++ .../backup/xtrabackup/xtrabackup_test.go | 11 ++++++++ .../xtrabackup_stream_test.go | 11 ++++++++ go/vt/mysqlctl/builtinbackupengine.go | 10 ++++--- go/vt/mysqlctl/compression.go | 27 ------------------- go/vt/mysqlctl/compression_test.go | 19 ------------- go/vt/wrangler/testlib/backup_test.go | 24 ++++++++++++++--- 8 files changed, 71 insertions(+), 53 deletions(-) diff --git a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go index 437435dfeb9..177c1a8b8ff 100644 --- a/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go +++ b/go/test/endtoend/backup/mysqlctld/backup_mysqlctld_test.go @@ -19,6 +19,8 @@ package mysqlctld import ( "testing" + "vitess.io/vitess/go/vt/mysqlctl" + backup "vitess.io/vitess/go/test/endtoend/backup/vtctlbackup" ) @@ -28,9 +30,18 @@ func TestBackupMysqlctld(t *testing.T) { } func TestBackupMysqlctldWithlz4Compression(t *testing.T) { + defer setDefaultCompressionFlag() cDetails := &backup.CompressionDetails{ BuiltinCompressor: "lz4", } backup.TestBackup(t, backup.Mysqlctld, "", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } + +func setDefaultCompressionFlag() { + *mysqlctl.BuiltinCompressor = "pgzip" + *mysqlctl.BuiltinDecompressor = "auto" + *mysqlctl.ExternalCompressorCmd = "" + *mysqlctl.ExternalCompressorExt = "" + *mysqlctl.ExternalDecompressorCmd = "" +} diff --git a/go/test/endtoend/backup/vtctlbackup/backup_test.go b/go/test/endtoend/backup/vtctlbackup/backup_test.go index 6c76bfba720..00e51c435e6 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_test.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_test.go @@ -18,6 +18,8 @@ package vtctlbackup import ( "testing" + + "vitess.io/vitess/go/vt/mysqlctl" ) // TestBackupMain - main tests backup using vtctl commands @@ -26,6 +28,7 @@ func TestBackupMain(t *testing.T) { } func TestBackupMainWithZstdCompression(t *testing.T) { + defer setDefaultCompressionFlag() cDetails := &CompressionDetails{ ExternalCompressorCmd: "zstd", ExternalCompressorExt: ".zst", @@ -34,3 +37,11 @@ func TestBackupMainWithZstdCompression(t *testing.T) { TestBackup(t, Backup, "", 0, cDetails, []string{"TestReplicaBackup", "TestPrimaryBackup"}) } + +func setDefaultCompressionFlag() { + *mysqlctl.BuiltinCompressor = "pgzip" + *mysqlctl.BuiltinDecompressor = "auto" + *mysqlctl.ExternalCompressorCmd = "" + *mysqlctl.ExternalCompressorExt = "" + *mysqlctl.ExternalDecompressorCmd = "" +} diff --git a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go index 03c60aafb9e..20ff7841506 100644 --- a/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go +++ b/go/test/endtoend/backup/xtrabackup/xtrabackup_test.go @@ -19,6 +19,8 @@ package vtctlbackup import ( "testing" + "vitess.io/vitess/go/vt/mysqlctl" + backup "vitess.io/vitess/go/test/endtoend/backup/vtctlbackup" ) @@ -28,6 +30,7 @@ func TestXtrabackup(t *testing.T) { } func TestXtrabackWithZstdCompression(t *testing.T) { + defer setDefaultCompressionFlag() cDetails := &backup.CompressionDetails{ ExternalCompressorCmd: "zstd", ExternalCompressorExt: ".zst", @@ -36,3 +39,11 @@ func TestXtrabackWithZstdCompression(t *testing.T) { backup.TestBackup(t, backup.XtraBackup, "tar", 0, cDetails, []string{"TestReplicaBackup"}) } + +func setDefaultCompressionFlag() { + *mysqlctl.BuiltinCompressor = "pgzip" + *mysqlctl.BuiltinDecompressor = "auto" + *mysqlctl.ExternalCompressorCmd = "" + *mysqlctl.ExternalCompressorExt = "" + *mysqlctl.ExternalDecompressorCmd = "" +} diff --git a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go index 89815923740..2e41c6bee5a 100644 --- a/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go +++ b/go/test/endtoend/backup/xtrabackupstream/xtrabackup_stream_test.go @@ -19,6 +19,8 @@ package vtctlbackup import ( "testing" + "vitess.io/vitess/go/vt/mysqlctl" + backup "vitess.io/vitess/go/test/endtoend/backup/vtctlbackup" ) @@ -28,9 +30,18 @@ func TestXtrabackupStream(t *testing.T) { } func TestXtrabackupStreamWithlz4Compression(t *testing.T) { + defer setDefaultCompressionFlag() cDetails := &backup.CompressionDetails{ BuiltinCompressor: "lz4", } backup.TestBackup(t, backup.XtraBackup, "xbstream", 8, cDetails, []string{"TestReplicaBackup"}) } + +func setDefaultCompressionFlag() { + *mysqlctl.BuiltinCompressor = "pgzip" + *mysqlctl.BuiltinDecompressor = "auto" + *mysqlctl.ExternalCompressorCmd = "" + *mysqlctl.ExternalCompressorExt = "" + *mysqlctl.ExternalDecompressorCmd = "" +} diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 8a92e16c19b..170c845d608 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -606,7 +606,11 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP // And restore the file. name := fmt.Sprintf("%v", i) params.Logger.Infof("Copying file %v: %v", name, fes[i].Name) - err := be.restoreFile(ctx, params, bh, &fes[i], bm.TransformHook, !bm.SkipCompress, bm.CompressionEngine, name) + var decompEngine = bm.CompressionEngine + if *BuiltinDecompressor != "auto" { + decompEngine = *BuiltinDecompressor + } + err := be.restoreFile(ctx, params, bh, &fes[i], bm.TransformHook, !bm.SkipCompress, decompEngine, name) if err != nil { rec.RecordError(vterrors.Wrapf(err, "can't restore file %v to %v", name, fes[i].Name)) } @@ -617,7 +621,7 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP } // restoreFile restores an individual file. -func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle, fe *FileEntry, transformHook string, compress bool, compressionEngine string, name string) (finalErr error) { +func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestoreParams, bh backupstorage.BackupHandle, fe *FileEntry, transformHook string, compress bool, deCompressionEngine string, name string) (finalErr error) { // Open the source file for reading. source, err := bh.ReadFile(ctx, name) if err != nil { @@ -665,7 +669,7 @@ func (be *BuiltinBackupEngine) restoreFile(ctx context.Context, params RestorePa if *ExternalDecompressorCmd != "" { decompressor, err = newExternalDecompressor(ctx, *ExternalDecompressorCmd, reader, params.Logger) } else { - decompressor, err = newBuiltinDecompressor(compressionEngine, reader, params.Logger) + decompressor, err = newBuiltinDecompressor(deCompressionEngine, reader, params.Logger) } if err != nil { return vterrors.Wrap(err, "can't create decompressor") diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index a12e73d86f2..e979f0ba7f4 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -63,7 +63,6 @@ func getEngineFromExtension(extension string) (string, error) { return eng[0], nil // we select the first supported engine in auto mode } } - return "", fmt.Errorf("%w %q", errUnsupportedCompressionExtension, extension) } @@ -75,7 +74,6 @@ func getExtensionFromEngine(engine string) (string, error) { } } } - return "", fmt.Errorf("%w %q", errUnsupportedCompressionEngine, engine) } @@ -84,7 +82,6 @@ func validateExternalCmd(cmd string) (string, error) { if cmd == "" { return "", errors.New("external command is empty") } - return exec.LookPath(cmd) } @@ -93,16 +90,13 @@ func prepareExternalCompressionCmd(ctx context.Context, cmdStr string) (*exec.Cm if err != nil { return nil, err } - if len(cmdArgs) < 1 { return nil, errors.New("external command is empty") } - cmdPath, err := validateExternalCmd(cmdArgs[0]) if err != nil { return nil, err } - return exec.CommandContext(ctx, cmdPath, cmdArgs[1:]...), nil } @@ -115,16 +109,12 @@ func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, return nil, vterrors.Wrap(err, "unable to start external command") } compressor := &externalCompressor{cmd: cmd} - cmd.Stdout = writer - cmdIn, err := cmd.StdinPipe() if err != nil { return nil, vterrors.Wrap(err, "cannot create external ompressor stdin pipe") } - compressor.stdin = cmdIn - cmdErr, err := cmd.StderrPipe() if err != nil { return nil, vterrors.Wrap(err, "cannot create external ompressor stderr pipe") @@ -136,7 +126,6 @@ func newExternalCompressor(ctx context.Context, cmdStr string, writer io.Writer, compressor.wg.Add(1) // we wait for the gorouting to finish when we call Close() on the writer go scanLinesToLogger("compressor stderr", cmdErr, logger, compressor.wg.Done) - return compressor, nil } @@ -149,18 +138,13 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade if err != nil { return nil, vterrors.Wrap(err, "unable to start external command") } - decompressor := &externalDecompressor{cmd: cmd} - cmd.Stdin = reader - cmdOut, err := cmd.StdoutPipe() if err != nil { return nil, vterrors.Wrap(err, "cannot create external decompressor stdout pipe") } - decompressor.stdout = cmdOut - cmdErr, err := cmd.StderrPipe() if err != nil { return nil, vterrors.Wrap(err, "cannot create external decompressor stderr pipe") @@ -172,7 +156,6 @@ func newExternalDecompressor(ctx context.Context, cmdStr string, reader io.Reade decompressor.wg.Add(1) // we wait for the gorouting to finish when we call Close() on the reader go scanLinesToLogger("decompressor stderr", cmdErr, logger, decompressor.wg.Done) - return decompressor, nil } @@ -186,10 +169,8 @@ func newBuiltinDecompressorFromExtension(extension, engine string, reader io.Rea if err != nil { return decompressor, err } - engine = eng } - return newBuiltinDecompressor(engine, reader, logger) } @@ -197,7 +178,6 @@ func newBuiltinDecompressorFromExtension(extension, engine string, reader io.Rea func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logger) (decompressor io.ReadCloser, err error) { if engine == "pargzip" { logger.Warningf("engine \"pargzip\" doesn't support decompression, using \"pgzip\" instead") - engine = "pgzip" } @@ -207,7 +187,6 @@ func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logg if err != nil { return nil, err } - decompressor = d case "lz4": decompressor = ioutil.NopCloser(lz4.NewReader(reader)) @@ -216,7 +195,6 @@ func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logg if err != nil { return nil, err } - decompressor = d.IOReadCloser() default: err = fmt.Errorf("Unkown decompressor engine: %q", engine) @@ -224,7 +202,6 @@ func newBuiltinDecompressor(engine string, reader io.Reader, logger logutil.Logg } logger.Infof("Decompressing backup using engine %q", engine) - return decompressor, err } @@ -236,7 +213,6 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger if err != nil { return compressor, vterrors.Wrap(err, "cannot create gzip compressor") } - gzip.SetConcurrency(*backupCompressBlockSize, *backupCompressBlocks) compressor = gzip case "pargzip": @@ -256,7 +232,6 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger if err != nil { return compressor, vterrors.Wrap(err, "cannot create zstd compressor") } - compressor = zst default: err = fmt.Errorf("Unkown compressor engine: %q", engine) @@ -264,7 +239,6 @@ func newBuiltinCompressor(engine string, writer io.Writer, logger logutil.Logger } logger.Infof("Compressing backup using engine %q", engine) - return } @@ -286,7 +260,6 @@ func (e *externalCompressor) Close() error { // wait for the stderr to finish reading as well e.wg.Wait() - return e.cmd.Wait() } diff --git a/go/vt/mysqlctl/compression_test.go b/go/vt/mysqlctl/compression_test.go index a9137be79ae..2c3d8827228 100644 --- a/go/vt/mysqlctl/compression_test.go +++ b/go/vt/mysqlctl/compression_test.go @@ -60,40 +60,33 @@ func TestGetExtensionFromEngine(t *testing.T) { func TestBuiltinCompressors(t *testing.T) { data := []byte("foo bar foobar") - logger := logutil.NewMemoryLogger() for _, engine := range []string{"pgzip", "lz4", "zstd"} { t.Run(engine, func(t *testing.T) { var compressed, decompressed bytes.Buffer - reader := bytes.NewReader(data) - compressor, err := newBuiltinCompressor(engine, &compressed, logger) if err != nil { t.Fatal(err) } - _, err = io.Copy(compressor, reader) if err != nil { t.Error(err) return } compressor.Close() - decompressor, err := newBuiltinDecompressor(engine, &compressed, logger) if err != nil { t.Error(err) return } - _, err = io.Copy(&decompressed, decompressor) if err != nil { t.Error(err) return } decompressor.Close() - if len(data) != len(decompressed.Bytes()) { t.Errorf("Different size of original (%d bytes) and uncompressed (%d bytes) data", len(data), len(decompressed.Bytes())) } @@ -124,9 +117,7 @@ func TestExternalCompressors(t *testing.T) { for _, tt := range tests { t.Run(tt.compress, func(t *testing.T) { var compressed, decompressed bytes.Buffer - reader := bytes.NewReader(data) - for _, cmd := range []string{tt.compress, tt.decompress} { cmdArgs := strings.Split(cmd, " ") @@ -135,40 +126,33 @@ func TestExternalCompressors(t *testing.T) { t.Skip("Command not available in this host:", err) } } - ctx, cancel := context.WithTimeout(context.Background(), time.Second*10) defer cancel() - compressor, err := newExternalCompressor(ctx, tt.compress, &compressed, logger) if err != nil { t.Error(err) return } - _, err = io.Copy(compressor, reader) if err != nil { t.Error(err) return } compressor.Close() - decompressor, err := newExternalDecompressor(ctx, tt.decompress, &compressed, logger) if err != nil { t.Error(err) return } - _, err = io.Copy(&decompressed, decompressor) if err != nil { t.Error(err) return } decompressor.Close() - if len(data) != len(decompressed.Bytes()) { t.Errorf("Different size of original (%d bytes) and uncompressed (%d bytes) data", len(data), len(decompressed.Bytes())) } - if !reflect.DeepEqual(data, decompressed.Bytes()) { t.Error("decompressed content differs from the original") } @@ -192,15 +176,12 @@ func TestValidateExternalCmd(t *testing.T) { for i, tt := range tests { t.Run(fmt.Sprintf("Test #%d", i+1), func(t *testing.T) { CmdName := tt.cmdName - path, err := validateExternalCmd(CmdName) - if tt.path != "" { if !strings.HasSuffix(path, tt.path) { t.Errorf("Expected path \"%s\" to include \"%s\"", path, tt.path) } } - if tt.errStr == "" { if err != nil { t.Errorf("Expected result \"%v\", got \"%v\"", "", err) diff --git a/go/vt/wrangler/testlib/backup_test.go b/go/vt/wrangler/testlib/backup_test.go index c3f1c43a083..d0f6476b31f 100644 --- a/go/vt/wrangler/testlib/backup_test.go +++ b/go/vt/wrangler/testlib/backup_test.go @@ -54,22 +54,34 @@ type compressionDetails struct { } func TestBackupRestore(t *testing.T) { - testBackupRestore(t, nil) + defer setDefaultCompressionFlag() + err := testBackupRestore(t, nil) + require.NoError(t, err) } // TODO: @rameez. I was expecting this test to fail but it turns out // we infer decompressor through compression engine in builtinEngine. // It is only in xtrabackup where we infer decompressor through extension & BuiltinDecompressor param. func TestBackupRestoreWithPargzip(t *testing.T) { + defer setDefaultCompressionFlag() cDetails := &compressionDetails{ BuiltinCompressor: "pargzip", BuiltinDecompressor: "lz4", } - testBackupRestore(t, cDetails) + err := testBackupRestore(t, cDetails) + require.ErrorContains(t, err, "lz4: bad magic number") } -func testBackupRestore(t *testing.T, cDetails *compressionDetails) { +func setDefaultCompressionFlag() { + *mysqlctl.BuiltinCompressor = "pgzip" + *mysqlctl.BuiltinDecompressor = "auto" + *mysqlctl.ExternalCompressorCmd = "" + *mysqlctl.ExternalCompressorExt = "" + *mysqlctl.ExternalDecompressorCmd = "" +} + +func testBackupRestore(t *testing.T, cDetails *compressionDetails) error { delay := discovery.GetTabletPickerRetryDelay() defer func() { discovery.SetTabletPickerRetryDelay(delay) @@ -238,7 +250,10 @@ func testBackupRestore(t *testing.T, cDetails *compressionDetails) { RelayLogInfoPath: path.Join(root, "relay-log.info"), } - require.NoError(t, destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */, time.Time{} /* backupTime */)) + err := destTablet.TM.RestoreData(ctx, logutil.NewConsoleLogger(), 0 /* waitForBackupInterval */, false /* deleteBeforeRestore */, time.Time{} /* backupTime */) + if err != nil { + return err + } // verify the full status require.NoError(t, destTablet.FakeMysqlDaemon.CheckSuperQueryList(), "destTablet.FakeMysqlDaemon.CheckSuperQueryList failed") assert.True(t, destTablet.FakeMysqlDaemon.Replicating) @@ -293,6 +308,7 @@ func testBackupRestore(t *testing.T, cDetails *compressionDetails) { assert.Equal(t, topodatapb.TabletType_PRIMARY, primary.Tablet.Type) assert.False(t, primary.FakeMysqlDaemon.Replicating) assert.True(t, primary.FakeMysqlDaemon.Running) + return nil } // TestBackupRestoreLagged tests the changes made in https://github.com/vitessio/vitess/pull/5000 From 130d0ad016409bba7a11bd08278d10e1a085f935 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Tue, 5 Jul 2022 15:31:29 -0700 Subject: [PATCH 18/21] fixing Upgrade/Downgrade test Signed-off-by: Rameez Sajwani --- go/vt/mysqlctl/builtinbackupengine.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index 170c845d608..aa1fdec4d8d 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -606,6 +606,11 @@ func (be *BuiltinBackupEngine) restoreFiles(ctx context.Context, params RestoreP // And restore the file. name := fmt.Sprintf("%v", i) params.Logger.Infof("Copying file %v: %v", name, fes[i].Name) + // For backward compatibility. Incase if Manifest is from N-1 binary + // then we assign the default value of compressionEngine. + if bm.CompressionEngine == "" { + bm.CompressionEngine = *BuiltinCompressor + } var decompEngine = bm.CompressionEngine if *BuiltinDecompressor != "auto" { decompEngine = *BuiltinDecompressor From 1fc44e0e6d358949296cd0dd6192014be619ed18 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Wed, 6 Jul 2022 12:11:24 -0700 Subject: [PATCH 19/21] Fix type & add summary under release notes Signed-off-by: Rameez Sajwani --- doc/releasenotes/15_0_0_summary.md | 28 ++++++++++++++++++++++++++++ go/flags/endtoend/vttablet.txt | 6 +++--- go/vt/mysqlctl/compression.go | 6 +++--- 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/doc/releasenotes/15_0_0_summary.md b/doc/releasenotes/15_0_0_summary.md index c9497df3321..36bcaa05a2c 100644 --- a/doc/releasenotes/15_0_0_summary.md +++ b/doc/releasenotes/15_0_0_summary.md @@ -4,4 +4,32 @@ ### New command line flags and behavior +#### Support for additional compressor and decompressor during backup & restore +We have introduced feature to use your own compressor & decompressor during backup & restore, instead of relying on default compressor. +There are some built-in compressors which you can use out-of-box instead of default 'pgzip'. We left it to client to evaluate which option +works better for their use-cases. Here are the flags for this feature + +- --builtin_compressor +- --builtin_decompressor +- --external_compressor +- --external_decompressor +- --external_compressor_extension +- --compression_level + +builtin_compressor as of today support the following compression out-of-box +- pgzip +- pargzip +- lz4 +- zstd + +If you want to use any of the builtin compressor, simply set one of the above value for "--builtin_compressor". You don't need to set +the builtin_decompressor flag in this case as we infer it automatically from the MANIFEST file. The default value for --builtin_decompressor +is set to "auto". + +If you are using custom command or external tool for compression then you need to use "--external_compressor/--external_decompressor" flag. +You need to provide complete command with arguments to these flags. "external_compressor_extension" needs to be set if you are using external +compressor. Leave the value of --builtin_compressor & --builtin-decompressor to their default values if you are using external compressor. +Please note that if you want the current behavior then you don't need to change anything in these flags. You can read more about backup & restore +[here] (https://vitess.io/docs/15.0/user-guides/operating-vitess/backup-and-restore/) + ### Online DDL changes \ No newline at end of file diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index d1c6ba7e6c8..156749fd43c 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/go/flags/endtoend/vttablet.txt @@ -414,11 +414,11 @@ Usage of vttablet: --enforce_strict_trans_tables If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES or STRICT_ALL_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database. (default true) --external_compressor string - command with arguments to use when decompressing a backup + command with arguments to use when compressing a backup --external_compressor_extension string - extension to use when using an external decompressor + extension to use when using an external compressor --external_decompressor string - command with arguments to use when compressing a backup + command with arguments to use when decompressing a backup --file_backup_storage_root string root directory for the file backup storage --filecustomrules string diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index e979f0ba7f4..6bf1be2bc5f 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -42,9 +42,9 @@ var ( BuiltinCompressor = flag.String("builtin_compressor", "pgzip", "builtin compressor engine to use") BuiltinDecompressor = flag.String("builtin_decompressor", "auto", "builtin decompressor engine to use") // use and external command to decompress the backups - ExternalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when decompressing a backup") - ExternalCompressorExt = flag.String("external_compressor_extension", "", "extension to use when using an external decompressor") - ExternalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when compressing a backup") + ExternalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when compressing a backup") + ExternalCompressorExt = flag.String("external_compressor_extension", "", "extension to use when using an external compressor") + ExternalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when decompressing a backup") errUnsupportedCompressionEngine = errors.New("unsupported engine") errUnsupportedCompressionExtension = errors.New("unsupported extension") From b6acb3e0d323ddf9c926a87f0c75f1cecb334a7e Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Wed, 6 Jul 2022 15:11:50 -0700 Subject: [PATCH 20/21] Fixing typos in summary Signed-off-by: Rameez Sajwani --- doc/releasenotes/15_0_0_summary.md | 42 +++++++++++++++--------------- go/flags/endtoend/vttablet.txt | 12 ++++----- go/vt/mysqlctl/compression.go | 12 ++++----- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/doc/releasenotes/15_0_0_summary.md b/doc/releasenotes/15_0_0_summary.md index c2e92f9ccdc..244786b743a 100644 --- a/doc/releasenotes/15_0_0_summary.md +++ b/doc/releasenotes/15_0_0_summary.md @@ -54,33 +54,33 @@ Please see the VDiff2 [documentation](https://vitess.io/docs/15.0/reference/vrep ### New command line flags and behavior -#### Support for additional compressor and decompressor during backup & restore -We have introduced feature to use your own compressor & decompressor during backup & restore, instead of relying on default compressor. -There are some built-in compressors which you can use out-of-box instead of default 'pgzip'. We left it to client to evaluate which option -works better for their use-cases. Here are the flags for this feature - -- --builtin_compressor -- --builtin_decompressor -- --external_compressor -- --external_decompressor -- --external_compressor_extension -- --compression_level - -builtin_compressor as of today support the following compression out-of-box +#### Support for additional compressors and decompressors during backup & restore +Backup/Restore now allow you many more options for compression and decompression instead of relying on the default compressor(pgzip). +There are some built-in compressors which you can use out-of-the-box. Users will need to evaluate which option works best for their +use-case. Here are the flags that control this feature + +- --builtin-compressor +- --builtin-decompressor +- --external-compressor +- --external-decompressor +- --external-compressor-extension +- --compression-level + +builtin compressor as of today supports the following options - pgzip - pargzip - lz4 - zstd -If you want to use any of the builtin compressor, simply set one of the above value for "--builtin_compressor". You don't need to set -the builtin_decompressor flag in this case as we infer it automatically from the MANIFEST file. The default value for --builtin_decompressor -is set to "auto". +If you want to use any of the builtin compressors, simply set one of the above values for `--builtin-compressor`. You don't need to set +the `--builtin-decompressor` flag in this case as we infer it automatically from the MANIFEST file. The default value for +`--builtin-decompressor` is `auto`. -If you are using custom command or external tool for compression then you need to use "--external_compressor/--external_decompressor" flag. -You need to provide complete command with arguments to these flags. "external_compressor_extension" needs to be set if you are using external -compressor. Leave the value of --builtin_compressor & --builtin-decompressor to their default values if you are using external compressor. -Please note that if you want the current behavior then you don't need to change anything in these flags. You can read more about backup & restore -[here] (https://vitess.io/docs/15.0/user-guides/operating-vitess/backup-and-restore/) +If you would like to use a custom command or external tool for compression/decompression then you need to provide the full command with +arguments to the `--external-compressor` and `--external-decompressor` flags. `--external-compressor-extension` flag also needs to be provided +so that compressed files are created with the correct extension. There is no need to override `--builtin-compressor` and `--builtin-decompressor` +when using an external compressor/decompressor. Please note that if you want the current behavior then you don't need to change anything +in these flags. You can read more about backup & restore [here] (https://vitess.io/docs/15.0/user-guides/operating-vitess/backup-and-restore/). ### Online DDL changes diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index 8949015a7c4..9b7de955354 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/go/flags/endtoend/vttablet.txt @@ -59,9 +59,9 @@ Usage of vttablet: (DEPRECATED) True if and only if the binlog streamer should use V3-style sharding, which doesn't require a preset sharding key column. (default true) --binlog_user string PITR restore parameter: username of binlog server. - --builtin_compressor string + --builtin-compressor string builtin compressor engine to use (default pgzip) - --builtin_decompressor string + --builtin-decompressor string builtin decompressor engine to use (default auto) --builtinbackup_mysqld_timeout duration how long to wait for mysqld to shutdown at the start of the backup (default 10m0s) @@ -71,7 +71,7 @@ Usage of vttablet: catch and ignore SIGPIPE on stdout and stderr if specified --ceph_backup_storage_config string Path to JSON config file for ceph backup storage (default ceph_backup_config.json) - --compression_level int + --compression-level int what level to pass to the compressor (default 1) --consul_auth_static_file string JSON File to read the topos/tokens from. @@ -409,11 +409,11 @@ Usage of vttablet: if this flag is true, vttablet will fail to start if a valid tableacl config does not exist --enforce_strict_trans_tables If true, vttablet requires MySQL to run with STRICT_TRANS_TABLES or STRICT_ALL_TABLES on. It is recommended to not turn this flag off. Otherwise MySQL may alter your supplied values before saving them to the database. (default true) - --external_compressor string + --external-compressor string command with arguments to use when compressing a backup - --external_compressor_extension string + --external-compressor-extension string extension to use when using an external compressor - --external_decompressor string + --external-decompressor string command with arguments to use when decompressing a backup --file_backup_storage_root string root directory for the file backup storage diff --git a/go/vt/mysqlctl/compression.go b/go/vt/mysqlctl/compression.go index 6bf1be2bc5f..c158f327391 100644 --- a/go/vt/mysqlctl/compression.go +++ b/go/vt/mysqlctl/compression.go @@ -37,14 +37,14 @@ import ( ) var ( - compressionLevel = flag.Int("compression_level", 1, "what level to pass to the compressor") + compressionLevel = flag.Int("compression-level", 1, "what level to pass to the compressor") // switch which compressor/decompressor to use - BuiltinCompressor = flag.String("builtin_compressor", "pgzip", "builtin compressor engine to use") - BuiltinDecompressor = flag.String("builtin_decompressor", "auto", "builtin decompressor engine to use") + BuiltinCompressor = flag.String("builtin-compressor", "pgzip", "builtin compressor engine to use") + BuiltinDecompressor = flag.String("builtin-decompressor", "auto", "builtin decompressor engine to use") // use and external command to decompress the backups - ExternalCompressorCmd = flag.String("external_compressor", "", "command with arguments to use when compressing a backup") - ExternalCompressorExt = flag.String("external_compressor_extension", "", "extension to use when using an external compressor") - ExternalDecompressorCmd = flag.String("external_decompressor", "", "command with arguments to use when decompressing a backup") + ExternalCompressorCmd = flag.String("external-compressor", "", "command with arguments to use when compressing a backup") + ExternalCompressorExt = flag.String("external-compressor-extension", "", "extension to use when using an external compressor") + ExternalDecompressorCmd = flag.String("external-decompressor", "", "command with arguments to use when decompressing a backup") errUnsupportedCompressionEngine = errors.New("unsupported engine") errUnsupportedCompressionExtension = errors.New("unsupported extension") From 2674adb9aa604a02da7567f65642c6fcf0c00a07 Mon Sep 17 00:00:00 2001 From: Rameez Sajwani Date: Wed, 6 Jul 2022 19:36:05 -0700 Subject: [PATCH 21/21] Fixing flag name typos Signed-off-by: Rameez Sajwani --- go/test/endtoend/backup/vtctlbackup/backup_utils.go | 10 +++++----- go/vt/mysqlctl/xtrabackupengine.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go/test/endtoend/backup/vtctlbackup/backup_utils.go b/go/test/endtoend/backup/vtctlbackup/backup_utils.go index 3d81cc40a88..32ba29b4296 100644 --- a/go/test/endtoend/backup/vtctlbackup/backup_utils.go +++ b/go/test/endtoend/backup/vtctlbackup/backup_utils.go @@ -229,19 +229,19 @@ func getCompressorArgs(cDetails *CompressionDetails) []string { } if cDetails.BuiltinCompressor != "" { - args = append(args, fmt.Sprintf("--builtin_compressor=%s", cDetails.BuiltinCompressor)) + args = append(args, fmt.Sprintf("--builtin-compressor=%s", cDetails.BuiltinCompressor)) } if cDetails.BuiltinDecompressor != "" { - args = append(args, fmt.Sprintf("--builtin_decompressor=%s", cDetails.BuiltinDecompressor)) + args = append(args, fmt.Sprintf("--builtin-decompressor=%s", cDetails.BuiltinDecompressor)) } if cDetails.ExternalCompressorCmd != "" { - args = append(args, fmt.Sprintf("--external_compressor=%s", cDetails.ExternalCompressorCmd)) + args = append(args, fmt.Sprintf("--external-compressor=%s", cDetails.ExternalCompressorCmd)) } if cDetails.ExternalCompressorExt != "" { - args = append(args, fmt.Sprintf("--external_compressor_extension=%s", cDetails.ExternalCompressorExt)) + args = append(args, fmt.Sprintf("--external-compressor-extension=%s", cDetails.ExternalCompressorExt)) } if cDetails.ExternalDecompressorCmd != "" { - args = append(args, fmt.Sprintf("--external_decompressor=%s", cDetails.ExternalDecompressorCmd)) + args = append(args, fmt.Sprintf("--external-decompressor=%s", cDetails.ExternalDecompressorCmd)) } return args diff --git a/go/vt/mysqlctl/xtrabackupengine.go b/go/vt/mysqlctl/xtrabackupengine.go index 9dc0e146d40..5abb0ae437a 100644 --- a/go/vt/mysqlctl/xtrabackupengine.go +++ b/go/vt/mysqlctl/xtrabackupengine.go @@ -141,7 +141,7 @@ func (be *XtrabackupEngine) ExecuteBackup(ctx context.Context, params BackupPara // an extension is required when using an external compressor if *backupStorageCompress && *ExternalCompressorCmd != "" && *ExternalCompressorExt == "" { return false, vterrors.New(vtrpc.Code_INVALID_ARGUMENT, - "xtrabackup_external_compressor_extension not provided when using an external compressor") + "xtrabackup-external-compressor-extension not provided when using an external compressor") } // use a mysql connection to detect flavor at runtime