From afd8de1f62188516b8bdc50c292262a20b33bd0c Mon Sep 17 00:00:00 2001 From: lance6716 Date: Thu, 30 May 2024 10:59:51 +0800 Subject: [PATCH] lightning: release the requirement for NewTargetInfoGetterImpl (#53651) --- lightning/pkg/importer/get_pre_info.go | 3 --- lightning/pkg/importer/get_pre_info_test.go | 14 ++++++++++---- pkg/lightning/backend/local/local.go | 13 +++++++++++-- 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/lightning/pkg/importer/get_pre_info.go b/lightning/pkg/importer/get_pre_info.go index 8bcb02d075ee3..5f744b7d822aa 100644 --- a/lightning/pkg/importer/get_pre_info.go +++ b/lightning/pkg/importer/get_pre_info.go @@ -143,9 +143,6 @@ func NewTargetInfoGetterImpl( case config.BackendTiDB: backendTargetInfoGetter = tidb.NewTargetInfoGetter(targetDB) case config.BackendLocal: - if pdHTTPCli == nil { - return nil, common.ErrUnknown.GenWithStack("pd HTTP client is required when using local backend") - } backendTargetInfoGetter = local.NewTargetInfoGetter(tls, targetDB, pdHTTPCli) default: return nil, common.ErrUnknownBackend.GenWithStackByArgs(cfg.TikvImporter.Backend) diff --git a/lightning/pkg/importer/get_pre_info_test.go b/lightning/pkg/importer/get_pre_info_test.go index bee6416adfcf5..ca5b0cfbf337a 100644 --- a/lightning/pkg/importer/get_pre_info_test.go +++ b/lightning/pkg/importer/get_pre_info_test.go @@ -753,16 +753,22 @@ func TestGetPreInfoEstimateSourceSize(t *testing.T) { } func TestGetPreInfoIsTableEmpty(t *testing.T) { - ctx := context.TODO() + ctx := context.Background() db, mock, err := sqlmock.New() require.NoError(t, err) lnConfig := config.NewConfig() lnConfig.TikvImporter.Backend = config.BackendLocal - _, err = NewTargetInfoGetterImpl(lnConfig, db, nil) - require.ErrorContains(t, err, "pd HTTP client is required when using local backend") - lnConfig.TikvImporter.Backend = config.BackendTiDB targetGetter, err := NewTargetInfoGetterImpl(lnConfig, db, nil) require.NoError(t, err) + mock.ExpectQuery("SELECT version()"). + WillReturnRows(sqlmock.NewRows([]string{"version()"}).AddRow("8.0.11-TiDB-v8.2.0-alpha-256-qweqweqw")) + err = targetGetter.CheckVersionRequirements(ctx) + require.ErrorContains(t, err, "pd HTTP client is required for component version check in local backend") + require.NoError(t, mock.ExpectationsWereMet()) + + lnConfig.TikvImporter.Backend = config.BackendTiDB + targetGetter, err = NewTargetInfoGetterImpl(lnConfig, db, nil) + require.NoError(t, err) require.Equal(t, lnConfig, targetGetter.cfg) mock.ExpectQuery("SELECT 1 FROM `test_db`.`test_tbl` USE INDEX\\(\\) LIMIT 1"). diff --git a/pkg/lightning/backend/local/local.go b/pkg/lightning/backend/local/local.go index 941e0893d2238..82974df80634e 100644 --- a/pkg/lightning/backend/local/local.go +++ b/pkg/lightning/backend/local/local.go @@ -262,8 +262,14 @@ type targetInfoGetter struct { pdHTTPCli pdhttp.Client } -// NewTargetInfoGetter creates an TargetInfoGetter with local backend implementation. -func NewTargetInfoGetter(tls *common.TLS, db *sql.DB, pdHTTPCli pdhttp.Client) backend.TargetInfoGetter { +// NewTargetInfoGetter creates an TargetInfoGetter with local backend +// implementation. `pdHTTPCli` should not be nil when need to check component +// versions in CheckRequirements. +func NewTargetInfoGetter( + tls *common.TLS, + db *sql.DB, + pdHTTPCli pdhttp.Client, +) backend.TargetInfoGetter { return &targetInfoGetter{ tls: tls, targetDB: db, @@ -293,6 +299,9 @@ func (g *targetInfoGetter) CheckRequirements(ctx context.Context, checkCtx *back if err := checkTiDBVersion(ctx, versionStr, localMinTiDBVersion, localMaxTiDBVersion); err != nil { return err } + if g.pdHTTPCli == nil { + return common.ErrUnknown.GenWithStack("pd HTTP client is required for component version check in local backend") + } if err := tikv.CheckPDVersion(ctx, g.pdHTTPCli, localMinPDVersion, localMaxPDVersion); err != nil { return err }