api: add new api to get the distribution of regions #9090

bufferflies · 2025-02-21T01:02:25Z

What problem does this PR solve?

Issue Number: Close #9089

What is changed and how does it work?

Check List

Tests

Unit test
Integration test

Code changes

Side effects

Related changes

Release note

None.

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot · 2025-02-21T01:02:27Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

Signed-off-by: 童剑 <[email protected]>

codecov · 2025-02-25T10:53:00Z

Codecov Report

Attention: Patch coverage is 75.91241% with 33 lines in your changes missing coverage. Please review.

Project coverage is 76.30%. Comparing base (2bbeb9c) to head (dee04ba).
Report is 19 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #9090      +/-   ##
==========================================
+ Coverage   76.25%   76.30%   +0.04%     
==========================================
  Files         468      468              
  Lines       71695    71850     +155     
==========================================
+ Hits        54673    54823     +150     
+ Misses      13610    13595      -15     
- Partials     3412     3432      +20

Flag	Coverage Δ
unittests	`76.30% <75.91%> (+0.04%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: 童剑 <[email protected]>

ti-chi-bot · 2025-02-27T07:28:14Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lhy1024

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [lhy1024]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2025-02-27T07:28:14Z

[LGTM Timeline notifier]

Timeline:

2025-02-27 07:28:14.407076522 +0000 UTC m=+513642.360234788: ☑️ agreed by lhy1024.

rleungx · 2025-02-27T10:29:27Z

client/http/api.go

@@ -38,6 +38,7 @@ const (
 	store                     = "/pd/api/v1/store"
 	Stores                    = "/pd/api/v1/stores"
 	StatsRegion               = "/pd/api/v1/stats/region"
+	DistributionRegion        = "/pd/api/v1/distributions/region"


How about "/pd/api/v1/regions/distribution"?

keep the style with the "/pd/api/v1/stats/region"

rleungx · 2025-03-05T08:41:26Z

server/cluster/cluster.go

@@ -2169,8 +2169,12 @@ func (c *RaftCluster) PutMetaCluster(meta *metapb.Cluster) error {
 }

 // GetRegionStatsByRange returns region statistics from cluster.
-func (c *RaftCluster) GetRegionStatsByRange(startKey, endKey []byte) *statistics.RegionStats {
-	return statistics.GetRegionStats(c.ScanRegions(startKey, endKey, -1))
+// if useHotFlow is true, the hot region statistics will be returned.


Why do we need it?

the new sql grama "show table t1 partition(p0,p1)" will use it.

rleungx · 2025-03-05T08:44:18Z

server/cluster/cluster.go

+	if useHotFlow {
+		return statistics.GetRegionStats(c.ScanRegions(startKey, endKey, -1), c, opts...)
+	}
+	return statistics.GetRegionStats(c.ScanRegions(startKey, endKey, -1), nil, opts...)


need pagination

rleungx · 2025-03-05T08:47:47Z

server/api/stats.go

+// @Produce  json
+// @Success  200  {object}  statistics.RegionStats
+// @Router   /distributions/region [get]
+func (h *statsHandler) GetRegionDistributions(w http.ResponseWriter, r *http.Request) {


It looks like this API is almost the same as GetRegionStatusByKeyRange, why not extend the existing API?

That's fine, but this api is already complex. It needs to consider three scenes such as:

the informations of the regions.

only needs the region count for spilt regions SQL or Tiflash.

the informations of the regions includes the hot statistics

rleungx · 2025-03-05T08:49:08Z

pkg/statistics/region.go

-		StoreLeaderKeys:  make(map[uint64]int64),
-		StorePeerSize:    make(map[uint64]int64),
-		StorePeerKeys:    make(map[uint64]int64),
+		StoreLeaderCount:     make(map[uint64]int),


Is there any OOM risk?

The length of the map is mostly small(less than 200). The API is primarily used by DBAs with a low frequent usage rate.

Signed-off-by: 童剑 <[email protected]>

add new api to get the distribution of regions

3d3b7be

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the distribution-api branch 2 times, most recently from 84099f2 to 66d6daa Compare February 25, 2025 08:57

ti-chi-bot bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 25, 2025

add unit test

e38effa

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the distribution-api branch from 66d6daa to e38effa Compare February 25, 2025 10:37

bufferflies force-pushed the distribution-api branch from 313f5b1 to 3bc276d Compare February 26, 2025 03:50

bufferflies marked this pull request as ready for review February 26, 2025 07:40

ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 26, 2025

bufferflies force-pushed the distribution-api branch from 3bc276d to 055401d Compare February 26, 2025 07:41

refactor

dee04ba

Signed-off-by: 童剑 <[email protected]>

bufferflies force-pushed the distribution-api branch from 055401d to dee04ba Compare February 26, 2025 08:10

lhy1024 approved these changes Feb 27, 2025

View reviewed changes

ti-chi-bot bot added the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Feb 27, 2025

ti-chi-bot bot added the approved label Feb 27, 2025

rleungx reviewed Feb 27, 2025

View reviewed changes

rleungx reviewed Mar 5, 2025

View reviewed changes

bufferflies added 2 commits March 6, 2025 11:48

resolve conflict

f4c0e20

Signed-off-by: 童剑 <[email protected]>

format

2e897e7

Signed-off-by: 童剑 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: add new api to get the distribution of regions #9090

api: add new api to get the distribution of regions #9090

bufferflies commented Feb 21, 2025

ti-chi-bot bot commented Feb 21, 2025

codecov bot commented Feb 25, 2025 •

edited

Loading

ti-chi-bot bot commented Feb 27, 2025

ti-chi-bot bot commented Feb 27, 2025

rleungx Feb 27, 2025

bufferflies Mar 4, 2025

rleungx Mar 5, 2025

bufferflies Mar 6, 2025

rleungx Mar 5, 2025

rleungx Mar 5, 2025

bufferflies Mar 6, 2025

rleungx Mar 5, 2025

bufferflies Mar 6, 2025

api: add new api to get the distribution of regions #9090

Are you sure you want to change the base?

api: add new api to get the distribution of regions #9090

Conversation

bufferflies commented Feb 21, 2025

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Feb 21, 2025

codecov bot commented Feb 25, 2025 • edited Loading

Codecov Report

ti-chi-bot bot commented Feb 27, 2025

ti-chi-bot bot commented Feb 27, 2025

[LGTM Timeline notifier]

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Feb 25, 2025 •

edited

Loading