cluster: move stats update into a better place #3901

rleungx · 2021-07-22T07:40:15Z

What problem does this PR solve?

Issue Number: Close #3286, close #3558, close #3199, ref #5648.

What is changed and how does it work?

Check List

Tests

Unit test

Release note

None

ti-chi-bot · 2021-07-22T07:40:16Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

JmPotato
disksing

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

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

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2021-07-22T07:59:09Z

Codecov Report

Attention: Patch coverage is 97.77778% with 2 lines in your changes missing coverage. Please review.

Project coverage is 75.56%. Comparing base (776003b) to head (d3f1da6).
Report is 1232 commits behind head on master.

Files	Patch %	Lines
server/cluster/cluster.go	92.85%	0 Missing and 1 partial ⚠️
server/cluster/coordinator.go	97.22%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3901      +/-   ##
==========================================
- Coverage   75.61%   75.56%   -0.05%     
==========================================
  Files         329      329              
  Lines       32965    32984      +19     
==========================================
  Hits        24925    24925              
- Misses       5896     5911      +15     
- Partials     2144     2148       +4

Flag	Coverage Δ
unittests	`75.56% <97.77%> (-0.05%)`	⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

nolouch · 2021-07-23T08:13:14Z

server/cluster/cluster.go

@@ -714,6 +714,11 @@ func (c *RaftCluster) processRegionHeartbeat(region *core.RegionInfo) error {
 		c.regionStats.Observe(region, c.getRegionStoresLocked(region))
 	}

+	// Updates the label level isolation statistics.
+	if c.labelLevelStats != nil {
+		c.labelLevelStats.Observe(region, c.getRegionStoresLocked(region), c.opt.GetLocationLabels())


I guess it‘s may cost many CPUs.

disksing · 2021-07-26T07:07:42Z

I think even if update statistics in heartbeat, it can still lose sync.

rleungx · 2021-11-23T09:10:07Z

I think even if update statistics in heartbeat, it can still lose sync.

Yes. I decide to leave it in region heartbeat process for now.

JmPotato · 2021-11-23T09:08:45Z

server/cluster/cluster.go

 	// persistLimitRetryTimes is used to reduce the probability of the persistent error
 	// since the once the store is add or remove, we shouldn't return an error even if the store limit is failed to persist.
+	defaultStatsJobLimit   = 10000


The comments above will mismatch.

server/cluster/coordinator_test.go

server/cluster/cluster_test.go

server/cluster/cluster.go

rleungx · 2021-11-23T11:15:17Z

/run-unit-tests

rleungx · 2021-11-24T03:47:29Z

/run-unit-tests

rleungx · 2021-11-24T05:30:09Z

/run-all-tests

Signed-off-by: Ryan Leung <[email protected]>

rleungx · 2022-12-12T04:53:28Z

/hold cancel

ti-chi-bot · 2023-01-12T13:51:44Z

@rleungx: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

rleungx added the component/cluster Cluster logic. label Jul 22, 2021

ti-chi-bot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 22, 2021

ti-chi-bot requested review from disksing and HunDunDM July 22, 2021 07:40

nolouch reviewed Jul 23, 2021

View reviewed changes

disksing removed their request for review August 24, 2021 03:07

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 16, 2021

rleungx force-pushed the fix-label-counter branch from 76ffb34 to 64ee28b Compare November 23, 2021 08:56

ti-chi-bot added do-not-merge/invalid-commit-message and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Nov 23, 2021

rleungx changed the title ~~cluster: fix the incorrect label counter~~ cluster: move stats update into a better place Nov 23, 2021

JmPotato reviewed Nov 23, 2021

View reviewed changes

rleungx requested review from JmPotato, nolouch and disksing November 23, 2021 09:35

JmPotato reviewed Nov 23, 2021

View reviewed changes

server/cluster/coordinator_test.go Show resolved Hide resolved

server/cluster/cluster_test.go Outdated Show resolved Hide resolved

server/cluster/cluster.go Outdated Show resolved Hide resolved

rleungx force-pushed the fix-label-counter branch from 661a4d2 to 1c542d4 Compare November 24, 2021 03:34

ti-chi-bot removed the do-not-merge/invalid-commit-message label Nov 24, 2021

rleungx force-pushed the fix-label-counter branch from a77a39f to 07d9360 Compare November 24, 2021 04:22

rleungx requested review from JmPotato and lhy1024 November 24, 2021 04:24

JmPotato approved these changes Nov 24, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 24, 2021

rleungx force-pushed the fix-label-counter branch 2 times, most recently from 81a0ff5 to 5bfd31c Compare December 28, 2021 03:26

rleungx force-pushed the fix-label-counter branch from ada3a48 to d1f8b55 Compare January 21, 2022 09:57

rleungx mentioned this pull request Jul 6, 2022

server/cluster: add regionState to record abnormal regions #5272

Open

rleungx mentioned this pull request Oct 28, 2022

Improve the performance of handling region heartbeats #5648

Closed

16 tasks

rleungx force-pushed the fix-label-counter branch from d1f8b55 to 305fb50 Compare October 28, 2022 05:40

ti-chi-bot added the do-not-merge/needs-linked-issue label Oct 28, 2022

rleungx force-pushed the fix-label-counter branch from 305fb50 to a8ae999 Compare October 28, 2022 08:39

nolouch removed request for HunDunDM and lhy1024 November 1, 2022 01:52

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 11, 2022

rleungx force-pushed the fix-label-counter branch from a8ae999 to fef450a Compare November 16, 2022 09:46

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 16, 2022

rleungx force-pushed the fix-label-counter branch from fef450a to d6dfc7a Compare November 16, 2022 09:47

rleungx force-pushed the fix-label-counter branch 2 times, most recently from 3c623f2 to f2127d1 Compare December 6, 2022 07:06

ti-chi-bot added release-note-none Denotes a PR that doesn't merit a release note. and removed do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Dec 9, 2022

rleungx force-pushed the fix-label-counter branch from f2127d1 to ceafc28 Compare December 9, 2022 04:58

rleungx added 3 commits December 12, 2022 11:05

async update region stats

98e509c

Signed-off-by: Ryan Leung <[email protected]>

resolve the conflict

8fa64b3

Signed-off-by: Ryan Leung <[email protected]>

resolve the conflict

1f13a24

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the fix-label-counter branch from ceafc28 to 48706ab Compare December 12, 2022 04:45

add monitor

d3f1da6

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the fix-label-counter branch from 48706ab to d3f1da6 Compare December 12, 2022 04:53

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 12, 2022

ti-chi-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cluster: move stats update into a better place #3901

cluster: move stats update into a better place #3901

rleungx commented Jul 22, 2021 •

edited

Loading

ti-chi-bot commented Jul 22, 2021 •

edited

Loading

codecov bot commented Jul 22, 2021 •

edited

Loading

nolouch Jul 23, 2021

disksing Jul 26, 2021

disksing commented Jul 26, 2021

rleungx commented Nov 23, 2021

JmPotato Nov 23, 2021

rleungx commented Nov 23, 2021

rleungx commented Nov 24, 2021

rleungx commented Nov 24, 2021

rleungx commented Dec 12, 2022

ti-chi-bot commented Jan 12, 2023

cluster: move stats update into a better place #3901

Are you sure you want to change the base?

cluster: move stats update into a better place #3901

Conversation

rleungx commented Jul 22, 2021 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Jul 22, 2021 • edited Loading

codecov bot commented Jul 22, 2021 • edited Loading

Codecov Report

nolouch Jul 23, 2021

Choose a reason for hiding this comment

disksing Jul 26, 2021

Choose a reason for hiding this comment

disksing commented Jul 26, 2021

rleungx commented Nov 23, 2021

JmPotato Nov 23, 2021

Choose a reason for hiding this comment

rleungx commented Nov 23, 2021

rleungx commented Nov 24, 2021

rleungx commented Nov 24, 2021

rleungx commented Dec 12, 2022

ti-chi-bot commented Jan 12, 2023

rleungx commented Jul 22, 2021 •

edited

Loading

ti-chi-bot commented Jul 22, 2021 •

edited

Loading

codecov bot commented Jul 22, 2021 •

edited

Loading