Skip to content

Delete range can not remove expected data#1783

Merged
flowbehappy merged 5 commits intopingcap:masterfrom
flowbehappy:enable_relevant_place.master
Apr 16, 2021
Merged

Delete range can not remove expected data#1783
flowbehappy merged 5 commits intopingcap:masterfrom
flowbehappy:enable_relevant_place.master

Conversation

@flowbehappy
Copy link
Contributor

@flowbehappy flowbehappy commented Apr 15, 2021 •
edited
Loading

What problem does this PR solve?

Problem Summary:

  1. Currently in DT engine, we only do place index for relevant query ranges in a segment. It can speed up individual region read, but the result of place index can not be shared between other region reads. It will eventually slow down the whole query processing if we have lots of cop requests.

  2. Some data failed to be removed after region peer migrated to other nodes.

What is changed and how it works?

What's Changed:

  1. This PR adds a new option dt_enable_relevant_place, and by default set to true, means enabling relevant place. The behavior is the same as the previous, but we can use the option to disable the relevant place.
  2. Fix bug: delete range can not remove expected data

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test

Side effects

Release note

  • Fix a potential issue that the DeleteRange in the storage engine failed to remove some data.

@flowbehappy flowbehappy added needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 type/bugfix This PR fixes a bug. needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 labels Apr 15, 2021
@flowbehappy flowbehappy added this to the v5.0.1 milestone Apr 15, 2021
@flowbehappy flowbehappy self-assigned this Apr 15, 2021
JaySon-Huang
JaySon-Huang reviewed Apr 15, 2021
View reviewed changes
JaySon-Huang
JaySon-Huang approved these changes Apr 15, 2021
View reviewed changes
Copy link
Contributor

@JaySon-Huang JaySon-Huang left a comment

Choose a reason for hiding this comment

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

LGTM
Please fill the Release note

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 15, 2021
@flowbehappy
Copy link
Contributor Author

flowbehappy commented Apr 16, 2021

/run-all-tests

@flowbehappy flowbehappy merged commit c72c331 into pingcap:master Apr 16, 2021
@flowbehappy flowbehappy deleted the enable_relevant_place.master branch April 16, 2021 08:43
@ti-srebot ti-srebot mentioned this pull request Apr 16, 2021
Merged
@ti-srebot
Copy link
Collaborator

ti-srebot commented Apr 16, 2021

cherry pick to release-4.0 in PR #1789

@ti-srebot ti-srebot mentioned this pull request Apr 16, 2021
Merged
@ti-srebot
Copy link
Collaborator

ti-srebot commented Apr 16, 2021

cherry pick to release-5.0 in PR #1790

flowbehappy pushed a commit that referenced this pull request Apr 16, 2021
@ti-srebot
Delete range can not remove expected data (#1783) #1790
f717fbe
flowbehappy added a commit that referenced this pull request Apr 16, 2021
@ti-srebot @flowbehappy
Delete range can not remove expected data (#1783) (#1789)
629002e 
* cherry pick #1783 to release-4.0

Signed-off-by: ti-srebot <ti-srebot@pingcap.com>

* fix compile errors

Co-authored-by: Flowyi <flowbehappy@gmail.com>
@ti-chi-bot
Copy link
Member

ti-chi-bot commented Sep 9, 2021

@flowbehappy: Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@ti-chi-bot ti-chi-bot added do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Sep 9, 2021
@lidezhu lidezhu mentioned this pull request Jul 5, 2022
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JaySon-Huang JaySon-Huang JaySon-Huang approved these changes

Assignees

@flowbehappy flowbehappy

Labels

do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. needs-cherry-pick-release-4.0 PR which needs to be cherry-picked to release-4.0 needs-cherry-pick-release-5.0 PR which needs to be cherry-picked to release-5.0 size/M Denotes a PR that changes 30-99 lines, ignoring generated files. status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.

Projects

None yet

Milestone

v5.0.1

Development

Successfully merging this pull request may close these issues.

4 participants

@flowbehappy @ti-srebot @ti-chi-bot @JaySon-Huang