From 2f48ef73e147adc6dcb398df74cde05325f4d5ee Mon Sep 17 00:00:00 2001 From: docsir Date: Mon, 18 Oct 2021 01:01:49 +0800 Subject: [PATCH 1/6] add err message for dumpling --- v4/export/dump.go | 8 ++++++++ v4/export/prepare_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/v4/export/dump.go b/v4/export/dump.go index ed36b207..66397315 100755 --- a/v4/export/dump.go +++ b/v4/export/dump.go @@ -60,6 +60,7 @@ func NewDumper(ctx context.Context, conf *Config) (*Dumper, error) { err := adjustConfig(conf, registerTLSConfig, validateSpecifiedSQL, + validateResolveAutoConsistency, adjustFileFormat) if err != nil { return nil, err @@ -1074,6 +1075,13 @@ func resolveAutoConsistency(d *Dumper) error { return nil } +func validateResolveAutoConsistency(conf *Config) error { + if conf.Consistency == consistencyTypeSnapshot || conf.Snapshot != "" { + return errors.New("can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + } + return nil +} + // tidbSetPDClientForGC is an initialization step of Dumper. func tidbSetPDClientForGC(d *Dumper) error { tctx, si, pool := d.tctx, d.conf.ServerInfo, d.dbHandle diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index bfe3fa9b..4d6b60b2 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -307,3 +307,32 @@ func TestConfigValidation(t *testing.T) { conf.FileType = "rand_str" require.EqualError(t, adjustFileFormat(conf), "unknown config.FileType 'rand_str'") } + +func TestValidateResolveAutoConsistency(t *testing.T) { + t.Parallel() + + conf := defaultConfigForTest(t) + conf.Consistency = consistencyTypeSnapshot + conf.Snapshot = "snapshot" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + + conf.Consistency = consistencyTypeSnapshot + conf.Snapshot = "" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + + conf.Consistency = consistencyTypeFlush + conf.Snapshot = "" + require.NoError(t, validateResolveAutoConsistency(conf)) + + conf.Consistency = consistencyTypeFlush + conf.Snapshot = "snapshot" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + + conf.Consistency = consistencyTypeNone + conf.Snapshot = "" + require.NoError(t, validateResolveAutoConsistency(conf)) + + conf.Consistency = consistencyTypeNone + conf.Snapshot = "snapshot" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") +} From 140490c20a037ad477b67d1e7ef9595218808237 Mon Sep 17 00:00:00 2001 From: docsir Date: Wed, 20 Oct 2021 10:30:51 +0800 Subject: [PATCH 2/6] modify test func --- v4/export/dump.go | 2 +- v4/export/prepare_test.go | 27 ++++++++++++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/v4/export/dump.go b/v4/export/dump.go index 66397315..b6be20c1 100755 --- a/v4/export/dump.go +++ b/v4/export/dump.go @@ -1076,7 +1076,7 @@ func resolveAutoConsistency(d *Dumper) error { } func validateResolveAutoConsistency(conf *Config) error { - if conf.Consistency == consistencyTypeSnapshot || conf.Snapshot != "" { + if conf.Consistency != consistencyTypeSnapshot && conf.Snapshot != "" { return errors.New("can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") } return nil diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index 4d6b60b2..9e62e111 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -313,19 +313,19 @@ func TestValidateResolveAutoConsistency(t *testing.T) { conf := defaultConfigForTest(t) conf.Consistency = consistencyTypeSnapshot - conf.Snapshot = "snapshot" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + conf.Snapshot = "123" + require.NoError(t, validateResolveAutoConsistency(conf)) conf.Consistency = consistencyTypeSnapshot conf.Snapshot = "" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + require.NoError(t, validateResolveAutoConsistency(conf)) conf.Consistency = consistencyTypeFlush conf.Snapshot = "" require.NoError(t, validateResolveAutoConsistency(conf)) conf.Consistency = consistencyTypeFlush - conf.Snapshot = "snapshot" + conf.Snapshot = "456" require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") conf.Consistency = consistencyTypeNone @@ -333,6 +333,23 @@ func TestValidateResolveAutoConsistency(t *testing.T) { require.NoError(t, validateResolveAutoConsistency(conf)) conf.Consistency = consistencyTypeNone - conf.Snapshot = "snapshot" + conf.Snapshot = "123" require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + + conf.Consistency = consistencyTypeAuto + conf.Snapshot = "" + require.NoError(t, validateResolveAutoConsistency(conf)) + + conf.Consistency = consistencyTypeAuto + conf.Snapshot = "133" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + + conf.Consistency = consistencyTypeLock + conf.Snapshot = "" + require.NoError(t, validateResolveAutoConsistency(conf)) + + conf.Consistency = consistencyTypeLock + conf.Snapshot = "122" + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + } From 30578878dd5bc3bf3a33cac428bd0519948ee1f1 Mon Sep 17 00:00:00 2001 From: docsir Date: Wed, 20 Oct 2021 10:51:58 +0800 Subject: [PATCH 3/6] change test cases --- v4/export/dump.go | 2 +- v4/export/prepare_test.go | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/v4/export/dump.go b/v4/export/dump.go index b6be20c1..c2710ad8 100755 --- a/v4/export/dump.go +++ b/v4/export/dump.go @@ -1077,7 +1077,7 @@ func resolveAutoConsistency(d *Dumper) error { func validateResolveAutoConsistency(conf *Config) error { if conf.Consistency != consistencyTypeSnapshot && conf.Snapshot != "" { - return errors.New("can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + return errors.Errorf("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: %s", conf.Consistency) } return nil } diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index 9e62e111..00bb0aef 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -326,7 +326,7 @@ func TestValidateResolveAutoConsistency(t *testing.T) { conf.Consistency = consistencyTypeFlush conf.Snapshot = "456" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush") conf.Consistency = consistencyTypeNone conf.Snapshot = "" @@ -334,7 +334,7 @@ func TestValidateResolveAutoConsistency(t *testing.T) { conf.Consistency = consistencyTypeNone conf.Snapshot = "123" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none") conf.Consistency = consistencyTypeAuto conf.Snapshot = "" @@ -342,7 +342,7 @@ func TestValidateResolveAutoConsistency(t *testing.T) { conf.Consistency = consistencyTypeAuto conf.Snapshot = "133" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto") conf.Consistency = consistencyTypeLock conf.Snapshot = "" @@ -350,6 +350,6 @@ func TestValidateResolveAutoConsistency(t *testing.T) { conf.Consistency = consistencyTypeLock conf.Snapshot = "122" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify both --consistency and --snapshot at the same time. snapshot consistency is not supported for this server") + require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock") } From 6612cdfe8a0d726e7d2b1cde685d17855f24566f Mon Sep 17 00:00:00 2001 From: docsir Date: Wed, 20 Oct 2021 13:47:48 +0800 Subject: [PATCH 4/6] use for loop and adjust fun location --- v4/export/dump.go | 6 ++--- v4/export/prepare_test.go | 56 ++++++++++++++++++++++++++++++--------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/v4/export/dump.go b/v4/export/dump.go index c2710ad8..dc2734db 100755 --- a/v4/export/dump.go +++ b/v4/export/dump.go @@ -60,7 +60,6 @@ func NewDumper(ctx context.Context, conf *Config) (*Dumper, error) { err := adjustConfig(conf, registerTLSConfig, validateSpecifiedSQL, - validateResolveAutoConsistency, adjustFileFormat) if err != nil { return nil, err @@ -72,7 +71,7 @@ func NewDumper(ctx context.Context, conf *Config) (*Dumper, error) { openSQLDB, detectServerInfo, resolveAutoConsistency, - + validateResolveAutoConsistency, tidbSetPDClientForGC, tidbGetSnapshot, tidbStartGCSavepointUpdateService, @@ -1075,7 +1074,8 @@ func resolveAutoConsistency(d *Dumper) error { return nil } -func validateResolveAutoConsistency(conf *Config) error { +func validateResolveAutoConsistency(d *Dumper) error { + conf := d.conf if conf.Consistency != consistencyTypeSnapshot && conf.Snapshot != "" { return errors.Errorf("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: %s", conf.Consistency) } diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index 00bb0aef..c342b49c 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -9,6 +9,7 @@ import ( "testing" tcontext "github.com/pingcap/dumpling/v4/context" + "github.com/pingcap/errors" "github.com/stretchr/testify/require" "github.com/DATA-DOG/go-sqlmock" @@ -311,45 +312,74 @@ func TestConfigValidation(t *testing.T) { func TestValidateResolveAutoConsistency(t *testing.T) { t.Parallel() - conf := defaultConfigForTest(t) - conf.Consistency = consistencyTypeSnapshot + conf1 := defaultConfigForTest(t) + d := &Dumper{conf: conf1} + conf := d.conf + + testCases := []struct { + confConsistency string + confSnapshot string + err error + }{ + {consistencyTypeAuto, "", nil}, + {consistencyTypeAuto, "123", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto")}, + {consistencyTypeFlush, "", nil}, + {consistencyTypeFlush, "456", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush")}, + {consistencyTypeLock, "", nil}, + {consistencyTypeLock, "789", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock")}, + {consistencyTypeSnapshot, "", nil}, + {consistencyTypeSnapshot, "456", nil}, + {consistencyTypeNone, "", nil}, + {consistencyTypeNone, "123", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none")}, + } + for _, testCase := range testCases { + conf.Consistency = testCase.confConsistency + conf.Snapshot = testCase.confSnapshot + if testCase.err == nil { + require.NoError(t, validateResolveAutoConsistency(d)) + } else { + require.EqualError(t, validateResolveAutoConsistency(d), testCase.err.Error()) + } + } + + /*conf.Consistency = consistencyTypeSnapshot conf.Snapshot = "123" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeSnapshot conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeFlush conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeFlush conf.Snapshot = "456" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush") + require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush") conf.Consistency = consistencyTypeNone conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeNone conf.Snapshot = "123" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none") + require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none") conf.Consistency = consistencyTypeAuto conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeAuto conf.Snapshot = "133" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto") + require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto") conf.Consistency = consistencyTypeLock conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(conf)) + require.NoError(t, validateResolveAutoConsistency(d)) conf.Consistency = consistencyTypeLock conf.Snapshot = "122" - require.EqualError(t, validateResolveAutoConsistency(conf), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock") - + require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock") + */ } From 43383428ae2c849bedb661a0c36429ccc047bfdc Mon Sep 17 00:00:00 2001 From: docsir Date: Wed, 20 Oct 2021 14:12:05 +0800 Subject: [PATCH 5/6] delete some explanatory notes --- v4/export/prepare_test.go | 41 --------------------------------------- 1 file changed, 41 deletions(-) diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index c342b49c..e8eb96ee 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -341,45 +341,4 @@ func TestValidateResolveAutoConsistency(t *testing.T) { require.EqualError(t, validateResolveAutoConsistency(d), testCase.err.Error()) } } - - /*conf.Consistency = consistencyTypeSnapshot - conf.Snapshot = "123" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeSnapshot - conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeFlush - conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeFlush - conf.Snapshot = "456" - require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush") - - conf.Consistency = consistencyTypeNone - conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeNone - conf.Snapshot = "123" - require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none") - - conf.Consistency = consistencyTypeAuto - conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeAuto - conf.Snapshot = "133" - require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto") - - conf.Consistency = consistencyTypeLock - conf.Snapshot = "" - require.NoError(t, validateResolveAutoConsistency(d)) - - conf.Consistency = consistencyTypeLock - conf.Snapshot = "122" - require.EqualError(t, validateResolveAutoConsistency(d), "can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock") - */ } From 6ca884414f36ec2747c908f8e15ef711a1a385a8 Mon Sep 17 00:00:00 2001 From: docsir Date: Wed, 20 Oct 2021 17:13:54 +0800 Subject: [PATCH 6/6] change test func --- v4/export/prepare_test.go | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go index e8eb96ee..cb02729b 100644 --- a/v4/export/prepare_test.go +++ b/v4/export/prepare_test.go @@ -9,7 +9,6 @@ import ( "testing" tcontext "github.com/pingcap/dumpling/v4/context" - "github.com/pingcap/errors" "github.com/stretchr/testify/require" "github.com/DATA-DOG/go-sqlmock" @@ -319,26 +318,26 @@ func TestValidateResolveAutoConsistency(t *testing.T) { testCases := []struct { confConsistency string confSnapshot string - err error + err bool }{ - {consistencyTypeAuto, "", nil}, - {consistencyTypeAuto, "123", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: auto")}, - {consistencyTypeFlush, "", nil}, - {consistencyTypeFlush, "456", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: flush")}, - {consistencyTypeLock, "", nil}, - {consistencyTypeLock, "789", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: lock")}, - {consistencyTypeSnapshot, "", nil}, - {consistencyTypeSnapshot, "456", nil}, - {consistencyTypeNone, "", nil}, - {consistencyTypeNone, "123", errors.New("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: none")}, + {consistencyTypeAuto, "", true}, + {consistencyTypeAuto, "123", false}, + {consistencyTypeFlush, "", true}, + {consistencyTypeFlush, "456", false}, + {consistencyTypeLock, "", true}, + {consistencyTypeLock, "789", false}, + {consistencyTypeSnapshot, "", true}, + {consistencyTypeSnapshot, "456", true}, + {consistencyTypeNone, "", true}, + {consistencyTypeNone, "123", false}, } for _, testCase := range testCases { conf.Consistency = testCase.confConsistency conf.Snapshot = testCase.confSnapshot - if testCase.err == nil { + if testCase.err == true { require.NoError(t, validateResolveAutoConsistency(d)) } else { - require.EqualError(t, validateResolveAutoConsistency(d), testCase.err.Error()) + require.EqualError(t, validateResolveAutoConsistency(d), fmt.Sprintf("can't specify --snapshot when --consistency isn't snapshot, resolved consistency: %s", conf.Consistency)) } } }