Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: Parameterize hotspot scheduling and adjust hot cache #2239

Merged
merged 17 commits into from
Mar 18, 2020

Conversation

nolouch
Copy link
Contributor

@nolouch nolouch commented Mar 13, 2020

Signed-off-by: nolouch [email protected]

What problem does this PR solve?

Improve stability and let hot region configurable.

What is changed and how it works?

  • fix the hot cache problem
  • parameterize the schedule
  • adjust the pendings

Check List

Tests

  • Unit test
  • Manual test
    Command: sysbench oltap_read_only (table size=2000000, threads=300)
    Before:
    image
    After:

image

@nolouch nolouch added component/schedule Scheduling logic. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Mar 13, 2020
Signed-off-by: nolouch <[email protected]>
@nolouch nolouch removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 16, 2020
@nolouch nolouch changed the title [WIP] *: Parameterize hotspot scheduling and adjust hot cache *: Parameterize hotspot scheduling and adjust hot cache Mar 16, 2020
@nolouch nolouch requested review from lhy1024 and disksing March 16, 2020 08:11
Signed-off-by: nolouch <[email protected]>
@@ -228,7 +230,7 @@ func (h *hotScheduler) gcRegionPendings() {
empty := true
for ty, op := range pendings {
if op != nil && op.IsEnd() {
if time.Now().After(op.GetCreateTime().Add(minRegionScheduleInterval)) {
if time.Now().After(op.GetCreateTime().Add(h.conf.GetMaxZombieDuration())) {
Copy link
Member

Choose a reason for hiding this comment

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

GetMaxZombieDuration?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it still influences the store, which should be consistent with the pendingSum.

tools/pd-ctl/pdctl/command/scheduler.go Outdated Show resolved Hide resolved
server/schedulers/hot_region_config.go Outdated Show resolved Hide resolved
server/schedulers/hot_region_config.go Outdated Show resolved Hide resolved
server/schedulers/hot_region_config.go Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #2239 into master will increase coverage by 0.03%.
The diff coverage is 78.26%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2239      +/-   ##
==========================================
+ Coverage   76.66%   76.69%   +0.03%     
==========================================
  Files         197      198       +1     
  Lines       21290    21442     +152     
==========================================
+ Hits        16321    16446     +125     
- Misses       3761     3784      +23     
- Partials     1208     1212       +4
Impacted Files Coverage Δ
server/statistics/hot_peer.go 100% <ø> (ø) ⬆️
server/statistics/hot_peer_cache.go 92.35% <100%> (+0.37%) ⬆️
tools/pd-ctl/pdctl/command/scheduler.go 68.07% <51.85%> (-1.18%) ⬇️
server/schedulers/hot_region_config.go 81.25% <81.25%> (ø)
server/schedulers/hot_region.go 81.91% <82%> (-1.29%) ⬇️
server/schedulers/shuffle_hot_region.go 63.96% <0%> (-9.91%) ⬇️
server/region_syncer/client.go 79.67% <0%> (-5.7%) ⬇️
server/schedulers/random_merge.go 61.19% <0%> (-2.99%) ⬇️
client/client.go 69.27% <0%> (-0.54%) ⬇️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 58a64fc...02a3f32. Read the comment docs.

@nolouch
Copy link
Contributor Author

nolouch commented Mar 17, 2020

ptal @disksing @rleungx

Signed-off-by: nolouch <[email protected]>
@nolouch
Copy link
Contributor Author

nolouch commented Mar 17, 2020

/rebuild

server/schedulers/hot_region_config.go Show resolved Hide resolved
server/schedulers/hot_region.go Show resolved Hide resolved
Copy link
Contributor

@lhy1024 lhy1024 left a comment

Choose a reason for hiding this comment

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

would you like to show test result?

@lhy1024 lhy1024 self-requested a review March 17, 2020 06:46
@nolouch
Copy link
Contributor Author

nolouch commented Mar 17, 2020

Hi, @lhy1024 I update the test result in the issue comment, you can check it now.

@nolouch
Copy link
Contributor Author

nolouch commented Mar 18, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 18, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 18, 2020

/run-all-tests

@sre-bot sre-bot merged commit dc08e2d into tikv:master Mar 18, 2020
@nolouch nolouch deleted the improve-hot-region branch March 18, 2020 04:29
nolouch added a commit to nolouch/pd that referenced this pull request Apr 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/schedule Scheduling logic. status/can-merge Indicates a PR has been approved by a committer. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants