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

replica selector refactor #1144

Closed
crazycs520 opened this issue Jan 26, 2024 · 2 comments · Fixed by #1142
Closed

replica selector refactor #1144

crazycs520 opened this issue Jan 26, 2024 · 2 comments · Fixed by #1142
Labels
enhancement New feature or request

Comments

@crazycs520
Copy link
Contributor

crazycs520 commented Jan 26, 2024

Background

Currently, the logic of replicaSelector is too complicated, and the code maintenance is a heavy burden. I guess no one can explain the common replica selection logic without looking at the code.

I think that the reason why replicaSelector is so complicated is because of the introduction of a state machine, which has many states and the transitions between states are also messy. The following is the state machine transition during the tidb_replica_read = leader strategy compiled by @zyguan, this is great, but still complicated, and this doesn't consider the situation when enable-forwarding is true, otherwise, it will be more complicated.

image

Task

This task is trying to refactor replicaSelector, and has the following targets:

  • Test replica selection strategies for various region error scenarios, and fix unreasonable behavior.
  • Less code, clearer logic, easier to understand.
  • Keep the replicaSelector logic/behavior the same as before.

Considering failure regression (since no one can guarantee that the new code is completely bug-free), the implementation is to introduce a new replicaSelectorV2, through the configuration file enable-replica-selector-v2 = true to use replicaSelectorV2 by default, set enable- replica-selector-v2 = false to fall back to using the older version of replicaSelector. After replicaSelectorV2 has gone through 2 major versions stably, we can start to consider deleting the old version of replicaSelector

@cfzjywxk
Copy link
Contributor

@crazycs520
Please add descriptions about problems and design details, so others could know why and how.

@cfzjywxk cfzjywxk added the enhancement New feature or request label Jan 30, 2024
@cfzjywxk
Copy link
Contributor

- Less code, clearer logic, easier to understand.
- Keep the replicaSelector logic/behavior the same as before.

@crazycs520

Simplifying the code is not a specific business objective. Please describe specific business objectives (such as unit test coverage of all state transitions and fixing unexpected issues) and prioritize the todo tasks accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants