Repository: pingcap/dumpling
Pull request #203 (2020-11-23 07:47:07)

refactor dump.go
<!--
Thank you for contributing to Dumpling! Please read the [CONTRIBUTING](https://github.com/pingcap/dumpling/blob/master/CONTRIBUTING.md) document **BEFORE** filing this PR.

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/pingcap/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md

You can use it with query parameters: https://github.com/pingcap/dumpling/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve? <!--add issue link with summary if exists-->

Refactor dumpling code to improve readability & maintainability.

### What is changed and how it works?
- Define the struct `Dumper` to wrap common arguments used by most functions.
- Extract the initialization steps into a few functions.
- Remove unreasonable fields in `Config` like `externalStorage`.

### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

 - Unit test
 - Integration test

Side effects

N/A

Related changes

N/A
 
### Release note
- No release note

<!-- bugfixes or new feature need a release note, must in the form of a list, such as

- support -T/--tables-list argument

or if no need to be included in the release note, just add the following line

- No release note
-->



Patch:
diff --git a/cmd/dumpling/main.go b/cmd/dumpling/main.go
index b545f15..7f26edc 100644
--- a/cmd/dumpling/main.go
+++ b/cmd/dumpling/main.go
@@ -62,7 +62,14 @@ func main() {
 	registry.MustRegister(prometheus.NewGoCollector())
 	export.RegisterMetrics(registry)
 	prometheus.DefaultGatherer = registry
-	err = export.Dump(context.Background(), conf)
+	dumper, err := export.NewDumper(context.Background(), conf)
+	if err != nil {
+		log.Error("dump failed error stack info", zap.Error(err))
+		fmt.Printf("\ncreate dumper failed: %s\n", err.Error())
+		os.Exit(1)
+	}
+	defer dumper.Close()
+	err = dumper.Dump()
 	if err != nil {
 		log.Error("dump failed error stack info", zap.Error(err))
 		fmt.Printf("\ndump failed: %s\n", err.Error())
diff --git a/v4/export/config.go b/v4/export/config.go
index 5c94cfc..5241439 100644
--- a/v4/export/config.go
+++ b/v4/export/config.go
@@ -13,9 +13,11 @@ import (
 
 	"github.com/coreos/go-semver/semver"
 	"github.com/docker/go-units"
+	"github.com/go-sql-driver/mysql"
 	"github.com/pingcap/br/pkg/storage"
 	"github.com/pingcap/errors"
 	filter "github.com/pingcap/tidb-tools/pkg/table-filter"
+	"github.com/pingcap/tidb-tools/pkg/utils"
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/spf13/pflag"
 	"go.uber.org/zap"
@@ -121,12 +123,11 @@ type Config struct {
 	EscapeBackslash          bool
 	DumpEmptyDatabase        bool
 	OutputFileTemplate       *template.Template `json:"-"`
+	TiDBMemQuotaQuery        uint64
 	SessionParams            map[string]interface{}
 
 	PosAfterConnect bool
 	Labels          prometheus.Labels `json:"-"`
-
-	ExternalStorage storage.ExternalStorage `json:"-"`
 }
 
 func DefaultConfig() *Config {
@@ -379,6 +380,10 @@ func (conf *Config) ParseFromFlags(flags *pflag.FlagSet) error {
 	if err != nil {
 		return errors.Trace(err)
 	}
+	conf.TiDBMemQuotaQuery, err = flags.GetUint64(flagTidbMemQuotaQuery)
+	if err != nil {
+		return errors.Trace(err)
+	}
 
 	if conf.Threads <= 0 {
 		return errors.Errorf("--threads is set to %d. It should be greater than 0", conf.Threads)
@@ -404,10 +409,6 @@ func (conf *Config) ParseFromFlags(flags *pflag.FlagSet) error {
 	if err != nil {
 		return errors.Trace(err)
 	}
-	tidbMemQuotaQuery, err := flags.GetUint64(flagTidbMemQuotaQuery)
-	if err != nil {
-		return errors.Trace(err)
-	}
 	outputFilenameFormat, err := flags.GetString(flagOutputFilenameTemplate)
 	if err != nil {
 		return errors.Trace(err)
@@ -431,8 +432,6 @@ func (conf *Config) ParseFromFlags(flags *pflag.FlagSet) error {
 		return errors.Trace(err)
 	}
 
-	conf.SessionParams[TiDBMemQuotaQueryName] = tidbMemQuotaQuery
-
 	if outputFilenameFormat == "" && conf.Sql != "" {
 		outputFilenameFormat = DefaultAnonymousOutputFileTemplateText
 	}
@@ -621,3 +620,50 @@ func init() {
 	serverTypeString[ServerTypeMariaDB] = "MariaDB"
 	serverTypeString[ServerTypeTiDB] = "TiDB"
 }
+
+func adjustConfig(conf *Config, fns ...func(*Config) error) error {
+	for _, f := range fns {
+		err := f(conf)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func initLogger(conf *Config) error {
+	if conf.Logger != nil {
+		log.SetAppLogger(conf.Logger)
+	} else {
+		err := log.InitAppLogger(&log.Config{
+			Level:  conf.LogLevel,
+			File:   conf.LogFile,
+			Format: conf.LogFormat,
+		})
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func registerTLSConfig(conf *Config) error {
+	if len(conf.Security.CAPath) > 0 {
+		tlsConfig, err := utils.ToTLSConfig(conf.Security.CAPath, conf.Security.CertPath, conf.Security.KeyPath)
+		if err != nil {
+			return err
+		}
+		err = mysql.RegisterTLSConfig("dumpling-tls-target", tlsConfig)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func validateSpecifiedSQL(conf *Config) error {
+	if conf.Sql != "" && conf.Where != "" {
+		return errors.New("can't specify both --sql and --where at the same time. Please try to combine them into --sql")
+	}
+	return nil
+}
diff --git a/v4/export/consistency.go b/v4/export/consistency.go
index e9357b3..f17d785 100644
--- a/v4/export/consistency.go
+++ b/v4/export/consistency.go
@@ -16,7 +16,6 @@ const (
 )
 
 func NewConsistencyController(ctx context.Context, conf *Config, session *sql.DB) (ConsistencyController, error) {
-	resolveAutoConsistency(conf)
 	conn, err := session.Conn(ctx)
 	if err != nil {
 		return nil, err
@@ -130,17 +129,3 @@ func (c *ConsistencyLockDumpingTables) PingContext(ctx context.Context) error {
 }
 
 const snapshotFieldIndex = 1
-
-func resolveAutoConsistency(conf *Config) {
-	if conf.Consistency != consistencyTypeAuto {
-		return
-	}
-	switch conf.ServerInfo.ServerType {
-	case ServerTypeTiDB:
-		conf.Consistency = consistencyTypeSnapshot
-	case ServerTypeMySQL, ServerTypeMariaDB:
-		conf.Consistency = consistencyTypeFlush
-	default:
-		conf.Consistency = consistencyTypeNone
-	}
-}
diff --git a/v4/export/dump.go b/v4/export/dump.go
index 3643467..5a5c370 100755
--- a/v4/export/dump.go
+++ b/v4/export/dump.go
@@ -3,13 +3,13 @@ package export
 import (
 	"context"
 	"database/sql"
-	"strconv"
 	"strings"
 	"time"
 
 	"github.com/pingcap/dumpling/v4/log"
 
 	_ "github.com/go-sql-driver/mysql"
+	"github.com/pingcap/br/pkg/storage"
 	"github.com/pingcap/br/pkg/summary"
 	"github.com/pingcap/br/pkg/utils"
 	"github.com/pingcap/errors"
@@ -19,123 +19,52 @@ import (
 	"golang.org/x/sync/errgroup"
 )
 
-func Dump(pCtx context.Context, conf *Config) (err error) {
-	if err = adjustConfig(pCtx, conf); err != nil {
-		return errors.Trace(err)
-	}
+type Dumper struct {
+	ctx       context.Context
+	conf      *Config
+	cancelCtx context.CancelFunc
 
-	go func() {
-		if conf.StatusAddr != "" {
-			err1 := startDumplingService(conf.StatusAddr)
-			if err1 != nil {
-				log.Error("dumpling stops to serving service", zap.Error(err1))
-			}
-		}
-	}()
+	extStore storage.ExternalStorage
+	dbHandle *sql.DB
 
-	pool, err := sql.Open("mysql", conf.GetDSN(""))
-	if err != nil {
-		return errors.Trace(err)
-	}
-	defer pool.Close()
+	tidbPDClientForGC pd.Client
+}
 
-	conf.ServerInfo, err = detectServerInfo(pool)
+func NewDumper(ctx context.Context, conf *Config) (*Dumper, error) {
+	ctx, cancelFn := context.WithCancel(ctx)
+	d := &Dumper{
+		ctx:       ctx,
+		conf:      conf,
+		cancelCtx: cancelFn,
+	}
+	err := adjustConfig(conf,
+		initLogger,
+		registerTLSConfig,
+		validateSpecifiedSQL)
 	if err != nil {
-		return errors.Trace(err)
-	}
-	resolveAutoConsistency(conf)
-	log.Info("finish config adjustment", zap.String("config", conf.String()))
-
-	ctx, cancel := context.WithCancel(pCtx)
-	defer cancel()
-
-	var (
-		doPdGC     bool
-		pdClient   pd.Client
-		snapshotTS uint64
-	)
-	if conf.ServerInfo.ServerType == ServerTypeTiDB {
-		if conf.ServerInfo.ServerVersion.Compare(*gcSafePointVersion) >= 0 {
-			pdAddrs, err := GetPdAddrs(pool)
-			if err != nil {
-				return err
-			}
-			if len(pdAddrs) > 0 {
-				doPdGC, err = checkSameCluster(ctx, pool, pdAddrs)
-				if err != nil {
-					log.Warn("meet error while check whether fetched pd addr and TiDB belongs to one cluster", zap.Error(err), zap.Strings("pdAddrs", pdAddrs))
-				} else if doPdGC {
-					pdClient, err = pd.NewClientWithContext(ctx, pdAddrs, pd.SecurityOption{})
-					if err != nil {
-						log.Warn("create pd client to control GC failed", zap.Error(err), zap.Strings("pdAddrs", pdAddrs))
-						doPdGC = false
-					}
-				}
-			}
-		}
-	} else {
-		delete(conf.SessionParams, TiDBMemQuotaQueryName)
-	}
-
-	snapshot := conf.Snapshot
-	if snapshot == "" && (doPdGC || conf.Consistency == consistencyTypeSnapshot) {
-		conn, err := pool.Conn(ctx)
-		if err != nil {
-			conn.Close()
-			return errors.Trace(err)
-		}
-		snapshot, err = getSnapshot(conn)
-		conn.Close()
-		if err != nil {
-			return err
-		}
-	}
-
-	if snapshot != "" {
-		if conf.ServerInfo.ServerType != ServerTypeTiDB {
-			return errors.New("snapshot consistency is not supported for this server")
-		}
-		snapshotTS, err = parseSnapshotToTSO(pool, snapshot)
-		if err != nil {
-			return err
-		}
-		if conf.Consistency == consistencyTypeSnapshot {
-			hasTiKV, err := CheckTiDBWithTiKV(pool)
-			if err != nil {
-				return err
-			}
-			if hasTiKV {
-				conf.SessionParams["tidb_snapshot"] = snapshot
-			}
-			// convert yyyy:mm:ss type snapshot to TSO if consistency is snapshot
-			snapshot = strconv.FormatUint(snapshotTS, 10)
-		} else {
-			// clear snapshot to use current master status for metadata (consistency lock)
-			snapshot = ""
-		}
+		return nil, err
 	}
+	err = runSteps(d,
+		createExternalStore,
+		startHttpService,
+		openSQLDB,
+		detectServerInfo,
+		resolveAutoConsistency,
 
-	if doPdGC {
-		go updateServiceSafePoint(ctx, pdClient, defaultDumpGCSafePointTTL, snapshotTS)
-	} else if conf.ServerInfo.ServerType == ServerTypeTiDB {
-		log.Warn("If the amount of data to dump is large, criteria: (data more than 60GB or dumped time more than 10 minutes)\n" +
-			"you'd better adjust the tikv_gc_life_time to avoid export failure due to TiDB GC during the dump process.\n" +
-			"Before dumping: run sql `update mysql.tidb set VARIABLE_VALUE = '720h' where VARIABLE_NAME = 'tikv_gc_life_time';` in tidb.\n" +
-			"After dumping: run sql `update mysql.tidb set VARIABLE_VALUE = '10m' where VARIABLE_NAME = 'tikv_gc_life_time';` in tidb.\n")
-	}
+		tidbSetPDClientForGC,
+		tidbGetSnapshot,
+		tidbStartGCSavepointUpdateService,
 
-	if newPool, err := resetDBWithSessionParams(pool, conf.GetDSN(""), conf.SessionParams); err != nil {
-		return errors.Trace(err)
-	} else {
-		pool = newPool
-		defer newPool.Close()
-	}
+		setSessionParam)
+	return d, err
+}
 
-	m := newGlobalMetadata(conf.ExternalStorage, snapshot)
-	// write metadata even if dump failed
+func (d *Dumper) Dump() (err error) {
+	ctx, conf, pool := d.ctx, d.conf, d.dbHandle
+	m := newGlobalMetadata(d.extStore, conf.Snapshot)
 	defer func() {
 		if err == nil {
-			m.writeGlobalMetaData(ctx)
+			_ = m.writeGlobalMetaData(ctx)
 		}
 	}()
 
@@ -214,7 +143,7 @@ func Dump(pCtx context.Context, conf *Config) (err error) {
 
 	failpoint.Inject("ConsistencyCheck", nil)
 
-	simpleWriter, err := NewSimpleWriter(conf)
+	simpleWriter, err := NewSimpleWriter(conf, d.extStore)
 	if err != nil {
 		return err
 	}
@@ -456,6 +385,170 @@ Loop:
 	return true, chunksIterArray, nil
 }
 
+func canRebuildConn(consistency string, trxConsistencyOnly bool) bool {
+	switch consistency {
+	case consistencyTypeLock, consistencyTypeFlush:
+		return !trxConsistencyOnly
+	case consistencyTypeSnapshot, consistencyTypeNone:
+		return true
+	default:
+		return false
+	}
+}
+
+func (d *Dumper) Close() error {
+	d.cancelCtx()
+	return d.dbHandle.Close()
+}
+
+func runSteps(d *Dumper, steps ...func(*Dumper) error) error {
+	for _, st := range steps {
+		err := st(d)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// createExternalStore is an initialization step of Dumper.
+func createExternalStore(d *Dumper) error {
+	ctx, conf := d.ctx, d.conf
+	b, err := storage.ParseBackend(conf.OutputDirPath, &conf.BackendOptions)
+	if err != nil {
+		return err
+	}
+	extStore, err := storage.Create(ctx, b, false)
+	if err != nil {
+		return err
+	}
+	d.extStore = extStore
+	return nil
+}
+
+// startHttpService is an initialization step of Dumper.
+func startHttpService(d *Dumper) error {
+	conf := d.conf
+	if conf.StatusAddr != "" {
+		go func() {
+			err := startDumplingService(conf.StatusAddr)
+			if err != nil {
+				log.Error("dumpling stops to serving service", zap.Error(err))
+			}
+		}()
+	}
+	return nil
+}
+
+// openSQLDB is an initialization step of Dumper.
+func openSQLDB(d *Dumper) error {
+	conf := d.conf
+	pool, err := sql.Open("mysql", conf.GetDSN(""))
+	if err != nil {
+		return errors.Trace(err)
+	}
+	d.dbHandle = pool
+	return nil
+}
+
+// detectServerInfo is an initialization step of Dumper.
+func detectServerInfo(d *Dumper) error {
+	db, conf := d.dbHandle, d.conf
+	versionStr, err := SelectVersion(db)
+	if err != nil {
+		conf.ServerInfo = ServerInfoUnknown
+		return err
+	}
+	conf.ServerInfo = ParseServerInfo(versionStr)
+	return nil
+}
+
+// resolveAutoConsistency is an initialization step of Dumper.
+func resolveAutoConsistency(d *Dumper) error {
+	conf := d.conf
+	if conf.Consistency != "auto" {
+		return nil
+	}
+	switch conf.ServerInfo.ServerType {
+	case ServerTypeTiDB:
+		conf.Consistency = "snapshot"
+	case ServerTypeMySQL, ServerTypeMariaDB:
+		conf.Consistency = "flush"
+	default:
+		conf.Consistency = "none"
+	}
+	return nil
+}
+
+// tidbSetPDClientForGC is an initialization step of Dumper.
+func tidbSetPDClientForGC(d *Dumper) error {
+	ctx, si, pool := d.ctx, d.conf.ServerInfo, d.dbHandle
+	if si.ServerType != ServerTypeTiDB ||
+		si.ServerVersion == nil ||
+		si.ServerVersion.Compare(*gcSafePointVersion) < 0 {
+		return nil
+	}
+	pdAddrs, err := GetPdAddrs(pool)
+	if err != nil {
+		return err
+	}
+	if len(pdAddrs) > 0 {
+		doPdGC, err := checkSameCluster(ctx, pool, pdAddrs)
+		if err != nil {
+			log.Warn("meet error while check whether fetched pd addr and TiDB belongs to one cluster", zap.Error(err), zap.Strings("pdAddrs", pdAddrs))
+		} else if doPdGC {
+			pdClient, err := pd.NewClientWithContext(ctx, pdAddrs, pd.SecurityOption{})
+			if err != nil {
+				log.Warn("create pd client to control GC failed", zap.Error(err), zap.Strings("pdAddrs", pdAddrs))
+			}
+			d.tidbPDClientForGC = pdClient
+		}
+	}
+	return nil
+}
+
+// tidbGetSnapshot is an initialization step of Dumper.
+func tidbGetSnapshot(d *Dumper) error {
+	conf, doPdGC := d.conf, d.tidbPDClientForGC != nil
+	consistency := conf.Consistency
+	pool, ctx := d.dbHandle, d.ctx
+	if conf.Snapshot == "" && (doPdGC || consistency == "snapshot") {
+		conn, err := pool.Conn(ctx)
+		if err != nil {
+			log.Warn("cannot get snapshot from TiDB", zap.Error(err))
+			return nil
+		}
+		snapshot, err := getSnapshot(conn)
+		_ = conn.Close()
+		if err != nil {
+			log.Warn("cannot get snapshot from TiDB", zap.Error(err))
+			return nil
+		}
+		conf.Snapshot = snapshot
+		return nil
+	}
+	return nil
+}
+
+// tidbStartGCSavepointUpdateService is an initialization step of Dumper.
+func tidbStartGCSavepointUpdateService(d *Dumper) error {
+	ctx, pool, conf := d.ctx, d.dbHandle, d.conf
+	snapshot, si := conf.Snapshot, conf.ServerInfo
+	if d.tidbPDClientForGC != nil {
+		snapshotTS, err := parseSnapshotToTSO(pool, snapshot)
+		if err != nil {
+			return err
+		}
+		go updateServiceSafePoint(ctx, d.tidbPDClientForGC, defaultDumpGCSafePointTTL, snapshotTS)
+	} else if si.ServerType == ServerTypeTiDB {
+		log.Warn("If the amount of data to dump is large, criteria: (data more than 60GB or dumped time more than 10 minutes)\n" +
+			"you'd better adjust the tikv_gc_life_time to avoid export failure due to TiDB GC during the dump process.\n" +
+			"Before dumping: run sql `update mysql.tidb set VARIABLE_VALUE = '720h' where VARIABLE_NAME = 'tikv_gc_life_time';` in tidb.\n" +
+			"After dumping: run sql `update mysql.tidb set VARIABLE_VALUE = '10m' where VARIABLE_NAME = 'tikv_gc_life_time';` in tidb.\n")
+	}
+	return nil
+}
+
 func updateServiceSafePoint(ctx context.Context, pdClient pd.Client, ttl int64, snapshotTS uint64) {
 	updateInterval := time.Duration(ttl/2) * time.Second
 	tick := time.NewTicker(updateInterval)
@@ -484,15 +577,35 @@ func updateServiceSafePoint(ctx context.Context, pdClient pd.Client, ttl int64,
 	}
 }
 
-func canRebuildConn(consistency string, trxConsistencyOnly bool) bool {
-	switch consistency {
-	case consistencyTypeLock, consistencyTypeFlush:
-		return !trxConsistencyOnly
-	case consistencyTypeSnapshot, consistencyTypeNone:
-		return true
-	default:
-		return false
+// setSessionParam is an initialization step of Dumper.
+func setSessionParam(d *Dumper) error {
+	conf, pool := d.conf, d.dbHandle
+	si := conf.ServerInfo
+	consistency, snapshot := conf.Consistency, conf.Snapshot
+	sessionParam := conf.SessionParams
+	if si.ServerType == ServerTypeTiDB {
+		sessionParam[TiDBMemQuotaQueryName] = conf.TiDBMemQuotaQuery
 	}
+	if snapshot != "" {
+		if si.ServerType != ServerTypeTiDB {
+			return errors.New("snapshot consistency is not supported for this server")
+		}
+		if consistency == consistencyTypeSnapshot {
+			hasTiKV, err := CheckTiDBWithTiKV(pool)
+			if err != nil {
+				return err
+			}
+			if hasTiKV {
+				sessionParam["tidb_snapshot"] = snapshot
+			}
+		}
+	}
+	if newPool, err := resetDBWithSessionParams(pool, conf.GetDSN(""), conf.SessionParams); err != nil {
+		return errors.Trace(err)
+	} else {
+		d.dbHandle = newPool
+	}
+	return nil
 }
 
 func shouldRedirectLog(conf *Config) bool {
diff --git a/v4/export/prepare.go b/v4/export/prepare.go
index 9273314..abfdfe9 100644
--- a/v4/export/prepare.go
+++ b/v4/export/prepare.go
@@ -1,17 +1,14 @@
 package export
 
 import (
-	"context"
 	"database/sql"
 	"fmt"
 	"regexp"
 	"strings"
 	"text/template"
 
-	"github.com/go-sql-driver/mysql"
 	"github.com/pingcap/dumpling/v4/log"
 	"github.com/pingcap/errors"
-	"github.com/pingcap/tidb-tools/pkg/utils"
 )
 
 const (
@@ -72,60 +69,6 @@ func ParseOutputFileTemplate(text string) (*template.Template, error) {
 	return template.Must(DefaultOutputFileTemplate.Clone()).Parse(text)
 }
 
-func adjustConfig(ctx context.Context, conf *Config) error {
-	// Init logger
-	if conf.Logger != nil {
-		log.SetAppLogger(conf.Logger)
-	} else {
-		err := log.InitAppLogger(&log.Config{
-			Level:  conf.LogLevel,
-			File:   conf.LogFile,
-			Format: conf.LogFormat,
-		})
-		if err != nil {
-			return err
-		}
-	}
-
-	// Register TLS config
-	if len(conf.Security.CAPath) > 0 {
-		tlsConfig, err := utils.ToTLSConfig(conf.Security.CAPath, conf.Security.CertPath, conf.Security.KeyPath)
-		if err != nil {
-			return err
-		}
-		err = mysql.RegisterTLSConfig("dumpling-tls-target", tlsConfig)
-		if err != nil {
-			return err
-		}
-	}
-
-	if conf.SessionParams == nil {
-		conf.SessionParams = make(map[string]interface{})
-	}
-	if conf.OutputFileTemplate == nil {
-		conf.OutputFileTemplate = DefaultOutputFileTemplate
-	}
-	if conf.Sql != "" && conf.Where != "" {
-		return errors.New("can't specify both --sql and --where at the same time. Please try to combine them into --sql")
-	}
-
-	s, err := conf.createExternalStorage(ctx)
-	if err != nil {
-		return err
-	}
-	conf.ExternalStorage = s
-
-	return nil
-}
-
-func detectServerInfo(db *sql.DB) (ServerInfo, error) {
-	versionStr, err := SelectVersion(db)
-	if err != nil {
-		return ServerInfoUnknown, err
-	}
-	return ParseServerInfo(versionStr), nil
-}
-
 func prepareDumpingDatabases(conf *Config, db *sql.Conn) ([]string, error) {
 	databases, err := ShowDatabases(db)
 	if len(conf.Databases) == 0 {
diff --git a/v4/export/writer.go b/v4/export/writer.go
index 48756ae..0598827 100644
--- a/v4/export/writer.go
+++ b/v4/export/writer.go
@@ -21,11 +21,15 @@ type Writer interface {
 }
 
 type SimpleWriter struct {
-	cfg *Config
+	cfg      *Config
+	extStore storage.ExternalStorage
 }
 
-func NewSimpleWriter(config *Config) (SimpleWriter, error) {
-	sw := SimpleWriter{cfg: config}
+func NewSimpleWriter(config *Config, externalStore storage.ExternalStorage) (SimpleWriter, error) {
+	sw := SimpleWriter{
+		cfg:      config,
+		extStore: externalStore,
+	}
 	return sw, nil
 }
 
@@ -34,7 +38,7 @@ func (f SimpleWriter) WriteDatabaseMeta(ctx context.Context, db, createSQL strin
 	if err != nil {
 		return err
 	}
-	return writeMetaToFile(ctx, db, createSQL, f.cfg.ExternalStorage, fileName+".sql", f.cfg.CompressType)
+	return writeMetaToFile(ctx, db, createSQL, f.extStore, fileName+".sql", f.cfg.CompressType)
 }
 
 func (f SimpleWriter) WriteTableMeta(ctx context.Context, db, table, createSQL string) error {
@@ -42,7 +46,7 @@ func (f SimpleWriter) WriteTableMeta(ctx context.Context, db, table, createSQL s
 	if err != nil {
 		return err
 	}
-	return writeMetaToFile(ctx, db, createSQL, f.cfg.ExternalStorage, fileName+".sql", f.cfg.CompressType)
+	return writeMetaToFile(ctx, db, createSQL, f.extStore, fileName+".sql", f.cfg.CompressType)
 }
 
 func (f SimpleWriter) WriteViewMeta(ctx context.Context, db, view, createTableSQL, createViewSQL string) error {
@@ -54,11 +58,11 @@ func (f SimpleWriter) WriteViewMeta(ctx context.Context, db, view, createTableSQ
 	if err != nil {
 		return err
 	}
-	err = writeMetaToFile(ctx, db, createTableSQL, f.cfg.ExternalStorage, fileNameTable+".sql", f.cfg.CompressType)
+	err = writeMetaToFile(ctx, db, createTableSQL, f.extStore, fileNameTable+".sql", f.cfg.CompressType)
 	if err != nil {
 		return err
 	}
-	return writeMetaToFile(ctx, db, createViewSQL, f.cfg.ExternalStorage, fileNameView+".sql", f.cfg.CompressType)
+	return writeMetaToFile(ctx, db, createViewSQL, f.extStore, fileNameView+".sql", f.cfg.CompressType)
 }
 
 type SQLWriter struct{ SimpleWriter }
@@ -75,7 +79,7 @@ func (f SQLWriter) WriteTableData(ctx context.Context, ir TableDataIR) (err erro
 	}
 
 	for {
-		fileWriter, tearDown := buildInterceptFileWriter(f.cfg.ExternalStorage, fileName, f.cfg.CompressType)
+		fileWriter, tearDown := buildInterceptFileWriter(f.extStore, fileName, f.cfg.CompressType)
 		err = WriteInsert(ctx, ir, fileWriter, f.cfg)
 		tearDown(ctx)
 		if err != nil {
@@ -178,7 +182,7 @@ func (f CSVWriter) WriteTableData(ctx context.Context, ir TableDataIR) (err erro
 	}
 
 	for {
-		fileWriter, tearDown := buildInterceptFileWriter(f.cfg.ExternalStorage, fileName, f.cfg.CompressType)
+		fileWriter, tearDown := buildInterceptFileWriter(f.extStore, fileName, f.cfg.CompressType)
 		err = WriteInsertInCsv(ctx, ir, fileWriter, f.cfg)
 		tearDown(ctx)
 		if err != nil {


Test patch:
diff --git a/v4/export/consistency_test.go b/v4/export/consistency_test.go
index 63eff2e..3764fa1 100644
--- a/v4/export/consistency_test.go
+++ b/v4/export/consistency_test.go
@@ -89,7 +89,8 @@ func (s *testConsistencySuite) TestResolveAutoConsistency(c *C) {
 	for _, x := range cases {
 		conf.Consistency = consistencyTypeAuto
 		conf.ServerInfo.ServerType = x.serverTp
-		resolveAutoConsistency(conf)
+		d := &Dumper{conf: conf}
+		c.Assert(resolveAutoConsistency(d), IsNil)
 		cmt := Commentf("server type %s", x.serverTp.String())
 		c.Assert(conf.Consistency, Equals, x.resolvedConsistency, cmt)
 	}
diff --git a/v4/export/prepare_test.go b/v4/export/prepare_test.go
index 95c78b1..97a42f8 100644
--- a/v4/export/prepare_test.go
+++ b/v4/export/prepare_test.go
@@ -110,16 +110,11 @@ func (s *testPrepareSuite) TestListAllTables(c *C) {
 	c.Assert(mock.ExpectationsWereMet(), IsNil)
 }
 
-func (s *testPrepareSuite) TestAdjustConfig(c *C) {
+func (s *testPrepareSuite) TestConfigValidation(c *C) {
 	conf := DefaultConfig()
 	conf.Where = "id < 5"
 	conf.Sql = "select * from t where id > 3"
-	c.Assert(adjustConfig(nil, conf), ErrorMatches, "can't specify both --sql and --where at the same time. Please try to combine them into --sql")
+	c.Assert(validateSpecifiedSQL(conf), ErrorMatches, "can't specify both --sql and --where at the same time. Please try to combine them into --sql")
 	conf.Where = ""
-	c.Assert(adjustConfig(nil, conf), IsNil)
-	conf.Sql = ""
-	conf.Rows = 5000
-	conf.FileSize = 5000
-	c.Assert(adjustConfig(nil, conf), IsNil)
-	c.Assert(conf.FileSize, Equals, uint64(5000))
+	c.Assert(validateSpecifiedSQL(conf), IsNil)
 }
diff --git a/v4/export/writer_test.go b/v4/export/writer_test.go
index bfd88f3..fa8b60c 100644
--- a/v4/export/writer_test.go
+++ b/v4/export/writer_test.go
@@ -3,6 +3,7 @@ package export
 import (
 	"context"
 	"database/sql/driver"
+	"github.com/pingcap/br/pkg/storage"
 	"io/ioutil"
 	"os"
 	"path"
@@ -14,16 +15,20 @@ var _ = Suite(&testWriterSuite{})
 
 type testWriterSuite struct{}
 
+func createExtStore(c *C, conf *Config) storage.ExternalStorage {
+	d := &Dumper{conf: conf}
+	c.Assert(createExternalStore(d), IsNil)
+	return d.extStore
+}
+
 func (s *testWriterSuite) TestWriteDatabaseMeta(c *C) {
 	dir := c.MkDir()
 	ctx := context.Background()
 
 	config := DefaultConfig()
 	config.OutputDirPath = dir
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 
-	writer, err := NewSimpleWriter(config)
+	writer, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	err = writer.WriteDatabaseMeta(ctx, "test", "CREATE DATABASE `test`")
 	c.Assert(err, IsNil)
@@ -41,10 +46,8 @@ func (s *testWriterSuite) TestWriteTableMeta(c *C) {
 
 	config := DefaultConfig()
 	config.OutputDirPath = dir
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 
-	writer, err := NewSimpleWriter(config)
+	writer, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	err = writer.WriteTableMeta(ctx, "test", "t", "CREATE TABLE t (a INT)")
 	c.Assert(err, IsNil)
@@ -62,10 +65,8 @@ func (s *testWriterSuite) TestWriteViewMeta(c *C) {
 
 	config := DefaultConfig()
 	config.OutputDirPath = dir
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 
-	writer, err := NewSimpleWriter(config)
+	writer, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	specCmt := "/*!40101 SET NAMES binary*/;\n"
 	createTableSQL := "CREATE TABLE `v`(\n`a` int\n)ENGINE=MyISAM;\n"
@@ -95,10 +96,8 @@ func (s *testWriterSuite) TestWriteTableData(c *C) {
 
 	config := DefaultConfig()
 	config.OutputDirPath = dir
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 
-	simpleWriter, err := NewSimpleWriter(config)
+	simpleWriter, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	writer := SQLWriter{SimpleWriter: simpleWriter}
 
@@ -141,8 +140,6 @@ func (s *testWriterSuite) TestWriteTableDataWithFileSize(c *C) {
 	config := DefaultConfig()
 	config.OutputDirPath = dir
 	config.FileSize = 50
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 	specCmts := []string{
 		"/*!40101 SET NAMES binary*/;",
 		"/*!40014 SET FOREIGN_KEY_CHECKS=0*/;",
@@ -151,7 +148,7 @@ func (s *testWriterSuite) TestWriteTableDataWithFileSize(c *C) {
 	config.FileSize += uint64(len(specCmts[1]) + 1)
 	config.FileSize += uint64(len("INSERT INTO `employees` VALUES\n"))
 
-	simpleWriter, err := NewSimpleWriter(config)
+	simpleWriter, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	writer := SQLWriter{SimpleWriter: simpleWriter}
 
@@ -198,8 +195,6 @@ func (s *testWriterSuite) TestWriteTableDataWithFileSizeAndRows(c *C) {
 	config.OutputDirPath = dir
 	config.FileSize = 50
 	config.Rows = 4
-	err := adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 	specCmts := []string{
 		"/*!40101 SET NAMES binary*/;",
 		"/*!40014 SET FOREIGN_KEY_CHECKS=0*/;",
@@ -208,7 +203,7 @@ func (s *testWriterSuite) TestWriteTableDataWithFileSizeAndRows(c *C) {
 	config.FileSize += uint64(len(specCmts[1]) + 1)
 	config.FileSize += uint64(len("INSERT INTO `employees` VALUES\n"))
 
-	simpleWriter, err := NewSimpleWriter(config)
+	simpleWriter, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	writer := SQLWriter{SimpleWriter: simpleWriter}
 
@@ -258,10 +253,8 @@ func (s *testWriterSuite) TestWriteTableDataWithStatementSize(c *C) {
 	var err error
 	config.OutputFileTemplate, err = ParseOutputFileTemplate("specified-name")
 	c.Assert(err, IsNil)
-	err = adjustConfig(ctx, config)
-	c.Assert(err, IsNil)
 
-	simpleWriter, err := NewSimpleWriter(config)
+	simpleWriter, err := NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
 	writer := SQLWriter{SimpleWriter: simpleWriter}
 
@@ -310,11 +303,12 @@ func (s *testWriterSuite) TestWriteTableDataWithStatementSize(c *C) {
 	// test specifying filename format
 	config.OutputFileTemplate, err = ParseOutputFileTemplate("{{.Index}}-{{.Table}}-{{fn .DB}}")
 	c.Assert(err, IsNil)
-	os.RemoveAll(config.OutputDirPath)
+	err = os.RemoveAll(config.OutputDirPath)
+	c.Assert(err, IsNil)
 	config.OutputDirPath, err = ioutil.TempDir("", "dumpling")
-	newStorage, err := config.createExternalStorage(context.Background())
+	simpleWriter, err = NewSimpleWriter(config, createExtStore(c, config))
 	c.Assert(err, IsNil)
-	config.ExternalStorage = newStorage
+	writer = SQLWriter{SimpleWriter: simpleWriter}
 
 	cases = map[string]string{
 		"000000000-employee-te%25%2Fst.sql": "/*!40101 SET NAMES binary*/;\n" +