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

let access-follower reload region on no candidate #901

Closed
wants to merge 1 commit into from

Conversation

zyguan
Copy link
Contributor

@zyguan zyguan commented Jul 24, 2023

Fix #879.

#843 only reload region when a failed store becomes reachable, thus can not handle issues like #879. This PR solve the issue by scheduling region reload when there is no candidate. To avoid reload region too frequently, it also limited the max number of pending reload regions.

With this PR, cross AZ traffic shall be reduced during the down-peer scheduling:

2023-07-24T05:31:31+00:00 graceful shutdown tikv@node-3:20161
2023-07-24T05:57:48+00:00 restart tikv@node-3:20161

2023-07-24_142024
2023-07-24_142107
2023-07-24_142123
2023-07-24_142041

if len(c.regionsNeedReload.regions) < maxPendingReloadRegions {
c.regionsNeedReload.regions = append(c.regionsNeedReload.regions, regionID)
} else {
c.regionsNeedReload.regions[rand.Intn(maxPendingReloadRegions)] = regionID
Copy link
Contributor

Choose a reason for hiding this comment

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

For this strategy, there seem to be starvation possibilities if scheduleReloadRegion keeps happening and no one can make progress.

Copy link
Contributor

Choose a reason for hiding this comment

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

And the region whose reload is canceled should be set to non-async status(set region.asyncReload to 0), unless it'll not be reloaded in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why not just ignore in this situation? Random indicates unpredictable...

@zyguan
Copy link
Contributor Author

zyguan commented Feb 2, 2024

#1122 is the better solution.

@zyguan zyguan closed this Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants