Remove getRegions call by lightning in physical import mode

[mittalrishabh]

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

When lightning runs in physical import mode, it runs this sql query

SELECT REGION_ID, APPROXIMATE_SIZE FROM information_schema.TIKV_REGION_STATUS WHERE TABLE_ID = ?

This call is made by each lightning process in parallel import mode and by multiple worker threads in a lightning process. It can potentially overload the PD if number of regions are too many. However, it appears that this call may be unnecessary since its purpose is to display a warning message and increment a counter in the SplitAndScatterRegionByRanges() function. Considering that lightning physical import always operates on a new table, splitting the new region should not pose any harm.

2. What did you expect to see? (Required)

Less number of get region calls to PD

3. What did you see instead (Required)

4. What is your TiDB version? (Required)

6.5

[AndreMouche]

It is a bug in this version since we have #39605 .
Meanwhile,As the @mittalrishabh mentioned, is it necessary to take the region of the new table here? Is there any room for optimization?

[AndreMouche]

@niubell Please take a look, thanks

[lance6716]

Yes, seems the correct behaviour is continue the loop for a skippedKey

tidb/br/pkg/lightning/backend/local/localhelper.go

Lines 343 to 364 in 9e20208

	sendLoop:
	for regionID, keys := range splitKeyMap {
	// if region not in tableRegionStats, that means this region is newly split, so
	// we can skip split it again.
	regionSize, ok := tableRegionStats[regionID]
	if !ok {
	log.FromContext(ctx).Warn("region stats not found", zap.Uint64("region", regionID))
	}
	if len(keys) == 1 && regionSize < regionSplitSize {
	skippedKeys++
	}
	select {
	case ch <- &splitInfo{region: regionMap[regionID], keys: keys}:
	case <-ctx.Done():
	// outer context is canceled, can directly return
	close(ch)
	return ctx.Err()
	case <-splitCtx.Done():
	// met critical error, stop process
	break sendLoop
	}
	}

But since we don't have a continue in code and everything works fine, I think remove the whole checking logic is acceptable. PTAL @niubell

[kennytm]

hi @niubell any update on this issue? thanks

[mittalrishabh]

I am going to create a PR for this issue. I have tested it internally.

[lance6716]

@mittalrishabh thanks!

[mittalrishabh]

filed PR #46202