From 98dd1c0575fee6b13952f34e45a11acb5a8b2834 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Tue, 11 Jul 2023 18:07:26 +0300 Subject: [PATCH 1/3] Incremental backup: accept GTID position without 'MySQL56/' flavor prefix Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/mysql/replication_position.go | 19 ++++++++++ go/mysql/replication_position_test.go | 18 ++++++++++ go/vt/mysqlctl/builtinbackupengine.go | 32 +++++++++-------- go/vt/mysqlctl/builtinbackupengine2_test.go | 39 +++++++++++++++++++++ 4 files changed, 93 insertions(+), 15 deletions(-) create mode 100644 go/vt/mysqlctl/builtinbackupengine2_test.go diff --git a/go/mysql/replication_position.go b/go/mysql/replication_position.go index d32d99e8011..d8b296a2484 100644 --- a/go/mysql/replication_position.go +++ b/go/mysql/replication_position.go @@ -145,6 +145,25 @@ func DecodePosition(s string) (rp Position, err error) { return ParsePosition(flav, gtid) } +// DecodePositionDefaultFlavor converts a string in the format returned by +// EncodePosition back into a Position value with the +// correct underlying flavor. If the string does not indicate a flavor, then the 'flavor' argument +// is used. For example: +// - DecodePositionDefaultFlavor("MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", "foo"): "MySQL56" explicitly indicated, this is the flavor. +// - DecodePositionDefaultFlavor("16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", "MySQL56"): No flavor indicated in `s`, therefore using "MySQL56" +func DecodePositionDefaultFlavor(s string, flavor string) (rp Position, err error) { + if s == "" { + return rp, nil + } + + flav, gtid, ok := strings.Cut(s, "/") + if !ok { + gtid = s + flav = flavor + } + return ParsePosition(flav, gtid) +} + // ParsePosition calls the parser for the specified flavor. func ParsePosition(flavor, value string) (rp Position, err error) { parser := gtidSetParsers[flavor] diff --git a/go/mysql/replication_position_test.go b/go/mysql/replication_position_test.go index 5bb2e5385d0..721fb166dac 100644 --- a/go/mysql/replication_position_test.go +++ b/go/mysql/replication_position_test.go @@ -272,6 +272,24 @@ func TestDecodePosition(t *testing.T) { } +func TestDecodePositionDefaultFlavor(t *testing.T) { + gtidSetParsers[Mysql56FlavorID] = func(s string) (GTIDSet, error) { + return ParseMysql56GTIDSet(s) + } + { + pos := "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" + rp, err := DecodePositionDefaultFlavor(pos, "foo") + assert.NoError(t, err) + assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", rp.GTIDSet.String()) + } + { + pos := "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" + rp, err := DecodePositionDefaultFlavor(pos, Mysql56FlavorID) + assert.NoError(t, err) + assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", rp.GTIDSet.String()) + } +} + func TestDecodePositionZero(t *testing.T) { input := "" want := Position{} diff --git a/go/vt/mysqlctl/builtinbackupengine.go b/go/vt/mysqlctl/builtinbackupengine.go index ca70e3eb908..8c2aeb0a4ff 100644 --- a/go/vt/mysqlctl/builtinbackupengine.go +++ b/go/vt/mysqlctl/builtinbackupengine.go @@ -208,6 +208,22 @@ func (be *BuiltinBackupEngine) ExecuteBackup(ctx context.Context, params BackupP return be.executeFullBackup(ctx, params, bh) } +// getIncrementalFromPosGTIDSet turns the given string into a valid Mysql56GTIDSet +func getIncrementalFromPosGTIDSet(incrementalFromPos string) (mysql.Mysql56GTIDSet, error) { + pos, err := mysql.DecodePositionDefaultFlavor(incrementalFromPos, mysql.Mysql56FlavorID) + if err != nil { + return nil, vterrors.Wrapf(err, "cannot decode position in incremental backup: %v", incrementalFromPos) + } + if !pos.MatchesFlavor(mysql.Mysql56FlavorID) { + return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "incremental backup only supports MySQL GTID positions. Got: %v", incrementalFromPos) + } + ifPosGTIDSet, ok := pos.GTIDSet.(mysql.Mysql56GTIDSet) + if !ok { + return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cannot get MySQL GTID value: %v", pos) + } + return ifPosGTIDSet, nil +} + // executeIncrementalBackup runs an incremental backup, based on given 'incremental_from_pos', which can be: // - A valid position // - "auto", indicating the incremental backup should begin with last successful backup end position. @@ -259,21 +275,7 @@ func (be *BuiltinBackupEngine) executeIncrementalBackup(ctx context.Context, par } // params.IncrementalFromPos is a string. We want to turn that into a MySQL GTID - getIncrementalFromPosGTIDSet := func() (mysql.Mysql56GTIDSet, error) { - pos, err := mysql.DecodePosition(params.IncrementalFromPos) - if err != nil { - return nil, vterrors.Wrapf(err, "cannot decode position in incremental backup: %v", params.IncrementalFromPos) - } - if !pos.MatchesFlavor(mysql.Mysql56FlavorID) { - return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "incremental backup only supports MySQL GTID positions. Got: %v", params.IncrementalFromPos) - } - ifPosGTIDSet, ok := pos.GTIDSet.(mysql.Mysql56GTIDSet) - if !ok { - return nil, vterrors.Errorf(vtrpc.Code_FAILED_PRECONDITION, "cannot get MySQL GTID value: %v", pos) - } - return ifPosGTIDSet, nil - } - backupFromGTIDSet, err := getIncrementalFromPosGTIDSet() + backupFromGTIDSet, err := getIncrementalFromPosGTIDSet(params.IncrementalFromPos) if err != nil { return false, err } diff --git a/go/vt/mysqlctl/builtinbackupengine2_test.go b/go/vt/mysqlctl/builtinbackupengine2_test.go new file mode 100644 index 00000000000..113f5121c77 --- /dev/null +++ b/go/vt/mysqlctl/builtinbackupengine2_test.go @@ -0,0 +1,39 @@ +/* +Copyright 2022 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_test is the blackbox tests for package mysqlctl. +package mysqlctl + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestGetIncrementalFromPosGTIDSet(t *testing.T) { + { + incrementalFromPos := "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" + gtidSet, err := getIncrementalFromPosGTIDSet(incrementalFromPos) + assert.NoError(t, err) + assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", gtidSet.String()) + } + { + incrementalFromPos := "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" + gtidSet, err := getIncrementalFromPosGTIDSet(incrementalFromPos) + assert.NoError(t, err) + assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", gtidSet.String()) + } +} From 53be1272a24286647153e055f214e6f061c4e02a Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Wed, 12 Jul 2023 07:34:36 +0300 Subject: [PATCH 2/3] testing invalid cases Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/mysqlctl/builtinbackupengine2_test.go | 50 ++++++++++++++++----- 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/go/vt/mysqlctl/builtinbackupengine2_test.go b/go/vt/mysqlctl/builtinbackupengine2_test.go index 113f5121c77..8aeb09540bb 100644 --- a/go/vt/mysqlctl/builtinbackupengine2_test.go +++ b/go/vt/mysqlctl/builtinbackupengine2_test.go @@ -24,16 +24,46 @@ import ( ) func TestGetIncrementalFromPosGTIDSet(t *testing.T) { - { - incrementalFromPos := "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" - gtidSet, err := getIncrementalFromPosGTIDSet(incrementalFromPos) - assert.NoError(t, err) - assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", gtidSet.String()) + tcases := []struct { + incrementalFromPos string + gtidSet string + expctError bool + }{ + { + "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", + "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", + false, + }, + { + "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", + "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", + false, + }, + { + "MySQL56/16b1039f-22b6-11ed-b765-0a43f95f28a3", + "", + true, + }, + { + "MySQL56/invalid", + "", + true, + }, + { + "16b1039f-22b6-11ed-b765-0a43f95f28a3", + "", + true, + }, } - { - incrementalFromPos := "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615" - gtidSet, err := getIncrementalFromPosGTIDSet(incrementalFromPos) - assert.NoError(t, err) - assert.Equal(t, "16b1039f-22b6-11ed-b765-0a43f95f28a3:1-615", gtidSet.String()) + for _, tcase := range tcases { + t.Run(tcase.incrementalFromPos, func(t *testing.T) { + gtidSet, err := getIncrementalFromPosGTIDSet(tcase.incrementalFromPos) + if tcase.expctError { + assert.Error(t, err) + } else { + assert.NoError(t, err) + assert.Equal(t, tcase.gtidSet, gtidSet.String()) + } + }) } } From 25c7c2172c4f09904489b191db15af5abd8143c9 Mon Sep 17 00:00:00 2001 From: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> Date: Thu, 13 Jul 2023 12:50:17 +0300 Subject: [PATCH 3/3] license year Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com> --- go/vt/mysqlctl/builtinbackupengine2_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/vt/mysqlctl/builtinbackupengine2_test.go b/go/vt/mysqlctl/builtinbackupengine2_test.go index 8aeb09540bb..29252392c7f 100644 --- a/go/vt/mysqlctl/builtinbackupengine2_test.go +++ b/go/vt/mysqlctl/builtinbackupengine2_test.go @@ -1,5 +1,5 @@ /* -Copyright 2022 The Vitess Authors. +Copyright 2023 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.