Skip to content

Commit 1746f02

Browse files
authored
dumpling: fix cannot dump data bug when dumpling fails to check has tikv (#40977)
close #40932
1 parent c34238b commit 1746f02

File tree

4 files changed

+72
-2
lines changed

4 files changed

+72
-2
lines changed

dumpling/export/BUILD.bazel

+1
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ go_test(
107107
"@com_github_data_dog_go_sqlmock//:go-sqlmock",
108108
"@com_github_go_sql_driver_mysql//:mysql",
109109
"@com_github_pingcap_errors//:errors",
110+
"@com_github_pingcap_failpoint//:failpoint",
110111
"@com_github_prometheus_client_golang//prometheus/collectors",
111112
"@com_github_stretchr_testify//require",
112113
"@org_golang_x_sync//errgroup",

dumpling/export/dump.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -1575,7 +1575,7 @@ func setSessionParam(d *Dumper) error {
15751575
d.L().Info("cannot check whether TiDB has TiKV, will apply tidb_snapshot by default. This won't affect dump process", log.ShortError(err))
15761576
}
15771577
if conf.ServerInfo.HasTiKV {
1578-
sessionParam["tidb_snapshot"] = snapshot
1578+
sessionParam[snapshotVar] = snapshot
15791579
}
15801580
}
15811581
}

dumpling/export/dump_test.go

+63
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ import (
1010

1111
"github.com/DATA-DOG/go-sqlmock"
1212
"github.com/coreos/go-semver/semver"
13+
"github.com/go-sql-driver/mysql"
1314
"github.com/pingcap/errors"
15+
"github.com/pingcap/failpoint"
1416
"github.com/pingcap/tidb/br/pkg/version"
1517
tcontext "github.com/pingcap/tidb/dumpling/context"
1618
"github.com/pingcap/tidb/parser"
@@ -289,3 +291,64 @@ func TestSetDefaultSessionParams(t *testing.T) {
289291
require.Equal(t, testCase.expectedParams, testCase.sessionParams)
290292
}
291293
}
294+
295+
func TestSetSessionParams(t *testing.T) {
296+
// case 1: fail to set tidb_snapshot, should return error with hint
297+
db, mock, err := sqlmock.New()
298+
require.NoError(t, err)
299+
defer func() {
300+
require.NoError(t, db.Close())
301+
}()
302+
303+
mock.ExpectQuery("SELECT @@tidb_config").
304+
WillReturnError(errors.New("mock error"))
305+
mock.ExpectQuery("SELECT COUNT\\(1\\) as c FROM MYSQL.TiDB WHERE VARIABLE_NAME='tikv_gc_safe_point'").
306+
WillReturnError(errors.New("mock error"))
307+
tikvErr := &mysql.MySQLError{
308+
Number: 1105,
309+
Message: "can not get 'tikv_gc_safe_point'",
310+
}
311+
mock.ExpectExec("SET SESSION tidb_snapshot").
312+
WillReturnError(tikvErr)
313+
314+
require.NoError(t, failpoint.Enable("github.com/pingcap/tidb/dumpling/export/SkipResetDB", "return(true)"))
315+
defer failpoint.Disable("github.com/pingcap/tidb/dumpling/export/SkipResetDB=return(true)")
316+
317+
tctx, cancel := tcontext.Background().WithLogger(appLogger).WithCancel()
318+
defer cancel()
319+
320+
conf := DefaultConfig()
321+
conf.ServerInfo = version.ServerInfo{
322+
ServerType: version.ServerTypeTiDB,
323+
HasTiKV: false,
324+
}
325+
conf.Snapshot = "439153276059648000"
326+
conf.Consistency = ConsistencyTypeSnapshot
327+
d := &Dumper{
328+
tctx: tctx,
329+
conf: conf,
330+
cancelCtx: cancel,
331+
dbHandle: db,
332+
}
333+
err = setSessionParam(d)
334+
require.ErrorContains(t, err, "consistency=none")
335+
336+
// case 2: fail to set other
337+
conf.ServerInfo = version.ServerInfo{
338+
ServerType: version.ServerTypeMySQL,
339+
HasTiKV: false,
340+
}
341+
conf.Snapshot = ""
342+
conf.Consistency = ConsistencyTypeFlush
343+
conf.SessionParams = map[string]interface{}{
344+
"mock": "UTC",
345+
}
346+
d.dbHandle = db
347+
mock.ExpectExec("SET SESSION mock").
348+
WillReturnError(errors.New("Unknown system variable mock"))
349+
mock.ExpectClose()
350+
mock.ExpectClose()
351+
352+
err = setSessionParam(d)
353+
require.NoError(t, err)
354+
}

dumpling/export/sql.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929

3030
const (
3131
orderByTiDBRowID = "ORDER BY `_tidb_rowid`"
32+
snapshotVar = "tidb_snapshot"
3233
)
3334

3435
type listTableType int
@@ -851,7 +852,9 @@ func resetDBWithSessionParams(tctx *tcontext.Context, db *sql.DB, cfg *mysql.Con
851852
s := fmt.Sprintf("SET SESSION %s = ?", k)
852853
_, err := db.ExecContext(tctx, s, pv)
853854
if err != nil {
854-
if isUnknownSystemVariableErr(err) {
855+
if k == snapshotVar {
856+
err = errors.Annotate(err, "fail to set snapshot for tidb, please set --consistency=none/--consistency=lock or fix snapshot problem")
857+
} else if isUnknownSystemVariableErr(err) {
855858
tctx.L().Info("session variable is not supported by db", zap.String("variable", k), zap.Reflect("value", v))
856859
continue
857860
}
@@ -876,6 +879,9 @@ func resetDBWithSessionParams(tctx *tcontext.Context, db *sql.DB, cfg *mysql.Con
876879
}
877880
cfg.Params[k] = s
878881
}
882+
failpoint.Inject("SkipResetDB", func(_ failpoint.Value) {
883+
failpoint.Return(db, nil)
884+
})
879885

880886
db.Close()
881887
c, err := mysql.NewConnector(cfg)

0 commit comments

Comments
 (0)