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

*: Make use of the upperBound of ticlient's kv_scan interface to ensure no overbound scan will happen (#8081) #8310

Merged
merged 6 commits into from
Nov 19, 2018

Conversation

MyonKeminta
Copy link
Contributor

@MyonKeminta MyonKeminta commented Nov 14, 2018

What problem does this PR solve?

This PR cherry-picks #8081 (09c6bff) to release-2.0 branch to fix the bug that scanning out-of-range may cause TiKV

This PR requires merging #8309 first.

What is changed and how it works?

This PR cherry-picks #8081 (09c6bff) to release-2.0 branch.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
    • Manually confirm that TiKV won't panic while executing add index or admin check table.

      1. Create two tables t1 and t2, insert some data in to them (values' length is large enough)
      2. Check t1 and t2's table_id (0x21 and 0x23)
      3. Delete some keys of t2 from default_cf by using RawKv API
        keys, _, err := cli.Scan([]byte("t\x80\x00\x00\x00\x00\x00\x00\xff\x23"), 3)
        if err != nil {
            panic(err)
        }
        
        for _, k := range keys {
            cli.Delete(k)
        }
      4. Try alter table t1 add index i1(id) and admin check table t1 in both current release-2.0 and this PR.

      Test result: In current release 2.0 branch TiKV will panic, while in this PR nothing strange happens.

Code changes

  • None of those listed

Side effects

  • Increased code complexity

Related changes

  • Need to be included in the release note

This change is Reviewable

@MyonKeminta
Copy link
Contributor Author

/run-all-tests pd=release-2.0 tikv=pr/3780 tidb-test=release-2.0

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

/run-unit-test

@zz-jason zz-jason changed the title [DNM] [release-2.0] cherry-pick: *: Make use of the upperBound of ticlient's kv_scan interface to ensure no overbound scan will happen (#8081) *: Make use of the upperBound of ticlient's kv_scan interface to ensure no overbound scan will happen (#8081) Nov 14, 2018
@MyonKeminta
Copy link
Contributor Author

/run-all-tests pd=release-2.0 tikv=release-2.0 tidb-test=release-2.0

@zz-jason
Copy link
Member

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 19, 2018
@MyonKeminta
Copy link
Contributor Author

@winkyao PTAL

Copy link
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@winkyao winkyao merged commit 51b4882 into pingcap:release-2.0 Nov 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/LGT1 Indicates that a PR has LGTM 1.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants