statistics: reduce unnecessary cpu time #5800

lhy1024 · 2022-12-26T10:42:43Z

Signed-off-by: lhy1024 [email protected]

What problem does this PR solve?

Issue Number: Close #5691

What is changed and how does it work?

cache Gauge.WithLabelValues
reduce log zap
improve collectExpiredItems and isOldColdPeer and justTransferLeader

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)

reduce 16% (from 110s to 92s)

Before:

After:

Release note

None.

ti-chi-bot · 2022-12-26T10:42:45Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch
rleungx

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.

Signed-off-by: lhy1024 <[email protected]>

codecov · 2022-12-26T18:56:56Z

Codecov Report

Base: 75.58% // Head: 75.57% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (966495e) compared to base (49fe21e).
Patch coverage: 79.01% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5800      +/-   ##
==========================================
- Coverage   75.58%   75.57%   -0.01%     
==========================================
  Files         338      338              
  Lines       33592    33603      +11     
==========================================
+ Hits        25390    25396       +6     
- Misses       6018     6028      +10     
+ Partials     2184     2179       -5

Flag	Coverage Δ
unittests	`75.57% <79.01%> (-0.01%)`	⬇️

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

Impacted Files	Coverage Δ
server/core/peer.go	`96.55% <ø> (-0.12%)`	⬇️
server/statistics/hot_peer.go	`79.41% <0.00%> (-20.59%)`	⬇️
server/statistics/kind.go	`37.86% <ø> (-6.80%)`	⬇️
server/statistics/hot_peer_cache.go	`95.00% <91.93%> (-1.93%)`	⬇️
pkg/mock/mockcluster/mockcluster.go	`94.25% <100.00%> (ø)`
server/statistics/hot_cache.go	`83.56% <100.00%> (-1.63%)`	⬇️
server/statistics/hot_cache_task.go	`95.74% <100.00%> (-0.09%)`	⬇️
pkg/storage/kv/etcd_kv.go	`79.71% <0.00%> (-5.80%)`	⬇️
pkg/utils/etcdutil/etcdutil.go	`84.88% <0.00%> (-3.49%)`	⬇️
... and 25 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Signed-off-by: lhy1024 <[email protected]>

lhy1024 · 2022-12-27T04:54:44Z

server/statistics/hot_peer_cache.go

-			if item != nil {
-				item.actionType = Remove
-				items = append(items, item)
+	if ids, ok := f.storesOfRegion[regionID]; ok {


length of f.storesOfRegion is less or equal than f.getAllStoreIDs

lhy1024 · 2022-12-27T04:55:48Z

server/statistics/hot_peer_cache.go

 	}
 	return t.rates
 }

 // gets the storeIDs, including old region and new region
 func (f *hotPeerCache) getAllStoreIDs(region *core.RegionInfo) []uint64 {
-	storeIDs := make(map[uint64]struct{})


the length of peers is enough small, map consume unnecessary

nolouch

lgtm

nolouch · 2023-01-06T07:13:41Z

ptal @rleungx

rleungx · 2023-01-06T08:04:58Z

server/statistics/hot_peer.go

-		zap.String("action-type", stat.actionType.String()),
-		zap.Time("last-transfer-leader-time", stat.lastTransferLeaderTime))
+func (stat *HotPeerStat) Log(str string) {
+	if log.GetLevel() <= zap.DebugLevel {


Why not just use log.Debug?

it will cost unnecessary cpu for zap

rleungx · 2023-01-06T08:10:06Z

server/statistics/hot_peer_cache_test.go

@@ -706,6 +703,40 @@ func TestHotPeerCacheTopN(t *testing.T) {
 	re.Equal(3200.0, cache.calcHotThresholds(1)[KeyDim])
 }

+func TestHotPeerCacheTopNLen(t *testing.T) {


This test is almost the same as TestHotPeerCacheTopNThreshold

Signed-off-by: lhy1024 <[email protected]>

rleungx

The rest LGTM.

server/statistics/hot_peer_cache_test.go

Signed-off-by: lhy1024 <[email protected]>

lhy1024 · 2023-01-09T09:27:21Z

/merge

ti-chi-bot · 2023-01-09T09:27:23Z

@lhy1024: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

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 ti-community-infra/tichi repository.

ti-chi-bot · 2023-01-09T09:27:24Z

This pull request has been accepted and is ready to merge.

Commit hash: 966495e

lhy1024 requested review from nolouch and HunDunDM December 26, 2022 10:42

ti-chi-bot added the release-note-none Denotes a PR that doesn't merit a release note. label Dec 26, 2022

lhy1024 added the component/statistics Statistics for scheduling. label Dec 26, 2022

statistics: reduce unnecessary cpu time

a45cc0b

Signed-off-by: lhy1024 <[email protected]>

lhy1024 force-pushed the hot_cpu branch from ef3dc5c to a45cc0b Compare December 26, 2022 18:50

lhy1024 added 2 commits December 27, 2022 12:35

reduce WithLabelValues of prometheus

bc355f6

Signed-off-by: lhy1024 <[email protected]>

replace map with slice

c0b6f63

Signed-off-by: lhy1024 <[email protected]>

lhy1024 commented Dec 27, 2022

View reviewed changes

nolouch approved these changes Dec 29, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 29, 2022

rleungx reviewed Jan 6, 2023

View reviewed changes

address comments

2349af4

Signed-off-by: lhy1024 <[email protected]>

rleungx approved these changes Jan 9, 2023

View reviewed changes

server/statistics/hot_peer_cache_test.go Outdated Show resolved Hide resolved

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 9, 2023

lhy1024 added 3 commits January 9, 2023 17:18

fix lint

4511e9d

Signed-off-by: lhy1024 <[email protected]>

address comments

e5e7edf

Signed-off-by: lhy1024 <[email protected]>

Merge branch 'master' into hot_cpu

966495e

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 9, 2023

ti-chi-bot merged commit f18aef7 into tikv:master Jan 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

statistics: reduce unnecessary cpu time #5800

statistics: reduce unnecessary cpu time #5800

lhy1024 commented Dec 26, 2022 •

edited

Loading

ti-chi-bot commented Dec 26, 2022 •

edited

Loading

codecov bot commented Dec 26, 2022 •

edited

Loading

lhy1024 Dec 27, 2022

lhy1024 Dec 27, 2022

nolouch left a comment

nolouch commented Jan 6, 2023

rleungx Jan 6, 2023

lhy1024 Jan 6, 2023

rleungx Jan 6, 2023

rleungx left a comment

lhy1024 commented Jan 9, 2023

ti-chi-bot commented Jan 9, 2023

ti-chi-bot commented Jan 9, 2023

statistics: reduce unnecessary cpu time #5800

statistics: reduce unnecessary cpu time #5800

Conversation

lhy1024 commented Dec 26, 2022 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot commented Dec 26, 2022 • edited Loading

codecov bot commented Dec 26, 2022 • edited Loading

Codecov Report

lhy1024 Dec 27, 2022

Choose a reason for hiding this comment

lhy1024 Dec 27, 2022

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

nolouch commented Jan 6, 2023

rleungx Jan 6, 2023

Choose a reason for hiding this comment

lhy1024 Jan 6, 2023

Choose a reason for hiding this comment

rleungx Jan 6, 2023

Choose a reason for hiding this comment

rleungx left a comment

Choose a reason for hiding this comment

lhy1024 commented Jan 9, 2023

ti-chi-bot commented Jan 9, 2023

ti-chi-bot commented Jan 9, 2023

lhy1024 commented Dec 26, 2022 •

edited

Loading

ti-chi-bot commented Dec 26, 2022 •

edited

Loading

codecov bot commented Dec 26, 2022 •

edited

Loading