lightning: release the requirement for NewTargetInfoGetterImpl (#53651)

pingcap · May 30, 2024 · afd8de1 · afd8de1
1 parent 22c41d0
commit afd8de1
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
 	}