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

coprocessor: let tiflash split range task consistent with tikv #14710

Merged

Conversation

lzmhhh123
Copy link
Contributor

What problem does this PR solve?

As before, we always set the full range for the region request of TiFlash. But there is some problem when we switch on the region merge. The retry of the merged region may cause the duplicate read. So TiFlash support the range in coprocessor and TiDB make the range split consistent with TiKV.

What is changed and how it works?

Make the range split of TiFlash region request consistent with TiKV.

Check List

Tests

  • Unit test
  • Integration test

Copy link
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

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

@lzmhhh123
Copy link
Contributor Author

Does the previous changes in /planner/core/find_best_task.go@master#L1085
need to be reverted?

I think we'd better not. The process of CBO would be affected.

@windtalker
Copy link
Contributor

Does the previous changes in /planner/core/find_best_task.go@master#L1085
need to be reverted?

I think we'd better not. The process of CBO would be affected.

Ok, but better to revert it in the future. And the changes in https://github.com/pingcap/tidb/blob/master/planner/core/explain.go#L153
should be reverted.

@lzmhhh123
Copy link
Contributor Author

Does the previous changes in /planner/core/find_best_task.go@master#L1085
need to be reverted?

I think we'd better not. The process of CBO would be affected.

Ok, but better to revert it in the future. And the changes in /planner/core/explain.go@master#L153
should be reverted.

OK.

@lzmhhh123 lzmhhh123 requested a review from a team as a code owner February 11, 2020 11:25
@ghost ghost requested review from francis0407 and removed request for a team February 11, 2020 11:25
Copy link
Contributor

@windtalker windtalker left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@lzmhhh123
Copy link
Contributor Author

/merge

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

sre-bot commented Feb 14, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Feb 14, 2020

@lzmhhh123 merge failed.

@lzmhhh123
Copy link
Contributor Author

/run-unit-test

@lzmhhh123 lzmhhh123 merged commit a5e0660 into pingcap:master Feb 14, 2020
@lzmhhh123 lzmhhh123 deleted the bug-fix/handle_correct_tiflash_range_in_cop branch February 14, 2020 07:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/coprocessor status/can-merge Indicates a PR has been approved by a committer. type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants