Skip to content

fix active_set bug in min tso scheduler#7693

Merged
ti-chi-bot[bot] merged 6 commits intopingcap:masterfrom
windtalker:min_tso_active_set
Jun 27, 2023
Merged

fix active_set bug in min tso scheduler#7693
ti-chi-bot[bot] merged 6 commits intopingcap:masterfrom
windtalker:min_tso_active_set

Conversation

@windtalker
Copy link
Contributor

@windtalker windtalker commented Jun 25, 2023

What problem does this PR solve?

Issue Number: close #7692

Problem Summary:

What is changed and how it works?

If active_set_soft_limit, only schdule the non min-tso task if the task belongs to an active query.

Check List

Tests

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

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Documentation

  • Affects user behaviors
  • Contains syntax changes
  • Contains variable changes
  • Contains experimental features
  • Changes MySQL compatibility

Release note

None

@windtalker
fix active_set bug in min tso
efa90cd 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. needs-cherry-pick-release-5.3 Type: Need cherry pick to release-5.3 needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 25, 2023
@SeaRise
Copy link
Contributor

SeaRise commented Jun 25, 2023

/run-integration-test

@SeaRise SeaRise self-requested a review June 26, 2023 02:51
SeaRise
SeaRise approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@SeaRise SeaRise left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add unit tests for MinTsoScheduler, in ComputeServerTest.
Add it or not, I think it's fine

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jun 26, 2023
yibin87
yibin87 approved these changes Jun 26, 2023
View reviewed changes
Copy link
Contributor

@yibin87 yibin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ti-chi-bot ti-chi-bot bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels Jun 26, 2023
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jun 26, 2023

[LGTM Timeline notifier]

Timeline:

  • 2023-06-26 02:57:18.30962738 +0000 UTC m=+590003.710877828: ☑️ agreed by SeaRise.
  • 2023-06-26 03:13:01.856065492 +0000 UTC m=+590947.257315936: ☑️ agreed by yibin87.

@windtalker
add ut
564088f 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 26, 2023
@windtalker
Copy link
Contributor Author

windtalker commented Jun 26, 2023

Maybe add unit tests for MinTsoScheduler, in ComputeServerTest. Add it or not, I think it's fine

I've added a ut in ComputeServerTest.

SeaRise
SeaRise reviewed Jun 26, 2023
View reviewed changes
@windtalker
address comments
25c04f3 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
SeaRise
SeaRise approved these changes Jun 26, 2023
View reviewed changes
dbms/src/Flash/tests/gtest_compute_server.cpp
std::this_thread::sleep_for(2s);
ASSERT_TRUE(TiFlashMetrics::instance().tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_active_queries_count).Value() == 2);
ASSERT_TRUE(TiFlashMetrics::instance().tiflash_task_scheduler.get(tiflash_task_scheduler_metrics::type_waiting_queries_count).Value() == 0);
for (size_t i = 1; i < running_queries.size(); i++)
Copy link
Contributor

@SeaRise SeaRise Jun 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some nits: i++
Others LGTM

@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Jun 26, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SeaRise, yibin87

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

The pull request process is described here

Details Needs approval from an approver in each of these files:

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

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-integration-test

@windtalker
fix tests
b801495 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-all-tests

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/rebuild

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-integration-test

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-integration-test

@windtalker
fix tests
d13acc9 
Signed-off-by: xufei <xufeixw@mail.ustc.edu.cn>
@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-all-tests

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/rebuild

@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-integration-test

@windtalker
fix ut
0f38218 
Signed-off-by: xufei <xufei@pingcap.com>
@windtalker
Copy link
Contributor Author

windtalker commented Jun 27, 2023

/run-all-tests

@ti-chi-bot ti-chi-bot bot merged commit 2bd982d into pingcap:master Jun 27, 2023
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 27, 2023
Closed
12 tasks
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 27, 2023

In response to a cherrypick label: new pull request created to branch release-5.3: #7707.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jun 27, 2023
@windtalker @ti-chi-bot
This is an automated cherry-pick of pingcap#7693
c70e5b6 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 27, 2023
Closed
12 tasks
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 27, 2023

In response to a cherrypick label: new pull request created to branch release-5.4: #7708.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jun 27, 2023
@windtalker @ti-chi-bot
This is an automated cherry-pick of pingcap#7693
9238ff3 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 27, 2023
Merged
12 tasks
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 27, 2023

In response to a cherrypick label: new pull request created to branch release-6.1: #7709.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jun 27, 2023
@windtalker @ti-chi-bot
This is an automated cherry-pick of pingcap#7693
71c8008 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 27, 2023
Merged
12 tasks
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 27, 2023

In response to a cherrypick label: new pull request created to branch release-6.5: #7710.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jun 27, 2023
@windtalker @ti-chi-bot
This is an automated cherry-pick of pingcap#7693
6a917aa 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Jun 27, 2023

In response to a cherrypick label: new pull request created to branch release-7.1: #7711.

ti-chi-bot pushed a commit to ti-chi-bot/tiflash that referenced this pull request Jun 27, 2023
@windtalker @ti-chi-bot
This is an automated cherry-pick of pingcap#7693
3a4e7be 
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
@ti-chi-bot ti-chi-bot mentioned this pull request Jun 27, 2023
Merged
12 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Jun 28, 2023
@ti-chi-bot
fix active_set bug in min tso scheduler (#7693) (#7710)
4f532a7 
close #7692
ti-chi-bot bot pushed a commit that referenced this pull request Jun 28, 2023
@ti-chi-bot
fix active_set bug in min tso scheduler (#7693) (#7709)
8765fe9 
close #7692
windtalker pushed a commit to windtalker/tiflash that referenced this pull request Jun 28, 2023
@ti-chi-bot @windtalker
fix active_set bug in min tso scheduler (pingcap#7693) (pingcap#7710)
3cc0fbe 
close pingcap#7692
ti-chi-bot bot pushed a commit that referenced this pull request Jun 28, 2023
@ti-chi-bot
fix active_set bug in min tso scheduler (#7693) (#7711)
562fad0 
close #7692
@windtalker windtalker mentioned this pull request Jun 28, 2023
Merged
12 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Jun 28, 2023
@windtalker
cherry-pick #7693 (#7712)
1433068
@windtalker windtalker deleted the min_tso_active_set branch September 20, 2023 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yibin87 yibin87 yibin87 approved these changes

+1 more reviewer

@SeaRise SeaRise SeaRise approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

approved lgtm needs-cherry-pick-release-5.3 Type: Need cherry pick to release-5.3 needs-cherry-pick-release-5.4 Should cherry pick this PR to release-5.4 branch. needs-cherry-pick-release-6.1 Should cherry pick this PR to release-6.1 branch. needs-cherry-pick-release-6.5 Should cherry pick this PR to release-6.5 branch. needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

active_set_soft_limit in MinTso scheduler does not work as expected.

4 participants

@windtalker @SeaRise @ti-chi-bot @yibin87