Optimize heartbeat - async process statistics #7898

nolouch · 2024-03-07T17:44:14Z

What problem does this PR solve?

Issue Number: Ref #7897

What is changed and how does it work?

Optimize heartbeat process
- Introduce `SyncRunner` and `AsyncRunner`
- Async the statics updates on subtree

Check List

Tests

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

Before:

After:

The duration of the pending state on tikvs has been reduced from 10 minutes to 3 minutes

Release note

None.

ti-chi-bot · 2024-03-07T17:44:17Z

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

ti-chi-bot · 2024-03-07T17:44:18Z

[REVIEW NOTIFICATION]

This pull request has not been approved.

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: nolouch <[email protected]>

codecov · 2024-03-21T03:06:36Z

Codecov Report

Merging #7898 (ac40311) into master (1331148) will increase coverage by 0.11%.
Report is 7 commits behind head on master.
The diff coverage is 82.42%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #7898      +/-   ##
==========================================
+ Coverage   73.47%   73.58%   +0.11%     
==========================================
  Files         436      437       +1     
  Lines       48349    48642     +293     
==========================================
+ Hits        35523    35793     +270     
- Misses       9765     9787      +22     
- Partials     3061     3062       +1

Flag	Coverage Δ
unittests	`73.58% <82.42%> (+0.11%)`	⬆️

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

HuSharp · 2024-04-07T03:49:38Z

pkg/ratelimit/runner.go

+			if opt.Limit != nil {
+				token, err = opt.Limit.Acquire(ctx)
+			}


Suggested change

if opt.Limit != nil {

token, err = opt.Limit.Acquire(ctx)

}

token, err = opt.Limit.Acquire(ctx)

HuSharp · 2024-04-07T03:52:44Z

pkg/mcs/scheduling/server/cluster.go

+		runner.RunTask(
+			ctx,
+			ratelimit.TaskOpts{
+				TaskName: "UpdateSubTree",


Should we put all the tasknames into a constant list?

HuSharp · 2024-04-07T06:15:57Z

pkg/ratelimit/runner.go

+
+func (s *AsyncRunner) recover() {
+	if r := recover(); r != nil {
+		log.Error("panic in runner", zap.Any("error", r))


Should we add more information? such as task name.

rleungx · 2024-04-07T06:33:32Z

pkg/core/region.go

 	return overlaps, nil
 }

+// CheckAndPutSupTree checks if the region is valid to put to the root, if valid then return error.
+// Usually used with CheckAndPutSubTree together.
+func (r *RegionsInfo) CheckAndPutSupTree(ctx context.Context, region *RegionInfo) ([]*RegionInfo, error) {


Suggested change

func (r *RegionsInfo) CheckAndPutSupTree(ctx context.Context, region *RegionInfo) ([]*RegionInfo, error) {

func (r *RegionsInfo) CheckAndPutSubTree(ctx context.Context, region *RegionInfo) ([]*RegionInfo, error) {

rleungx · 2024-04-07T06:42:51Z

pkg/ratelimit/concurrency_limiter.go

-	l.mu.RLock()
-	defer l.mu.RUnlock()
+func (l *ConcurrencyLimiter) getCurrent() uint64 {
+	l.mu.Lock()


why change it?

rleungx · 2024-04-07T06:43:46Z

pkg/ratelimit/concurrency_limiter.go

+
+// GetWaitingTasksNum returns the number of waiting tasks.
+func (l *ConcurrencyLimiter) GetWaitingTasksNum() uint64 {
+	l.mu.Lock()


rleungx · 2024-04-07T06:44:32Z

pkg/ratelimit/runner.go

@@ -0,0 +1,114 @@
+// Copyright 2022 TiKV Project Authors.


Suggested change

// Copyright 2022 TiKV Project Authors.

// Copyright 2024 TiKV Project Authors.

rleungx · 2024-04-07T06:53:17Z

pkg/ratelimit/runner.go

+	if opt.Limit != nil && atomic.LoadInt64(&s.numTasks) >= int64(s.maxPendingTasks) {
+		return ErrMaxWaitingTasksExceeded
+	}
+	s.addTask(1)


Will it keep adding without executing the defer?

nolouch · 2024-04-09T06:55:38Z

will split this pr, close.

ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 7, 2024

ti-chi-bot bot requested review from disksing and rleungx March 7, 2024 17:44

ti-chi-bot bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Mar 7, 2024

nolouch force-pushed the optmize-heartbeat branch 2 times, most recently from 193c2b5 to b3c59b1 Compare March 20, 2024 10:15

nolouch marked this pull request as ready for review March 20, 2024 10:18

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 Mar 20, 2024

nolouch changed the title ~~[Draft] Optimize heartbeat - async process statistics~~ Optimize heartbeat - async process statistics Mar 20, 2024

nolouch force-pushed the optmize-heartbeat branch from b3c59b1 to e334d81 Compare March 20, 2024 10:32

nolouch requested review from JmPotato and CabinfeverB and removed request for disksing March 20, 2024 10:34

nolouch force-pushed the optmize-heartbeat branch 3 times, most recently from 0ef7f0d to 25e0c99 Compare March 21, 2024 02:43

*: optimize heartbeat process with asyun runner

ac40311

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

nolouch force-pushed the optmize-heartbeat branch from 25e0c99 to ac40311 Compare March 21, 2024 02:50

nolouch mentioned this pull request Mar 21, 2024

Enhance heartbeat processing capabilities #7897

Closed

7 tasks

nolouch mentioned this pull request Apr 3, 2024

pkg/ratelimit: introduce an executor that can run with a limiter #8024

Merged

HuSharp reviewed Apr 7, 2024

View reviewed changes

rleungx reviewed Apr 7, 2024

View reviewed changes

nolouch closed this Apr 9, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize heartbeat - async process statistics #7898

Optimize heartbeat - async process statistics #7898

nolouch commented Mar 7, 2024 •

edited

Loading

ti-chi-bot bot commented Mar 7, 2024

ti-chi-bot bot commented Mar 7, 2024

codecov bot commented Mar 21, 2024

HuSharp Apr 7, 2024

HuSharp Apr 7, 2024

HuSharp Apr 7, 2024

rleungx Apr 7, 2024

rleungx Apr 7, 2024

rleungx Apr 7, 2024

rleungx Apr 7, 2024

rleungx Apr 7, 2024

nolouch commented Apr 9, 2024

	func (r RegionsInfo) CheckAndPutSupTree(ctx context.Context, region RegionInfo) ([]*RegionInfo, error) {
	func (r RegionsInfo) CheckAndPutSubTree(ctx context.Context, region RegionInfo) ([]*RegionInfo, error) {

	// Copyright 2022 TiKV Project Authors.
	// Copyright 2024 TiKV Project Authors.

Optimize heartbeat - async process statistics #7898

Optimize heartbeat - async process statistics #7898

Conversation

nolouch commented Mar 7, 2024 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented Mar 7, 2024

ti-chi-bot bot commented Mar 7, 2024

codecov bot commented Mar 21, 2024

Codecov Report

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

nolouch commented Apr 9, 2024

nolouch commented Mar 7, 2024 •

edited

Loading