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

store/mockstore: Fix that DeleteRange didn't encode keys. #6264

Merged
merged 5 commits into from
Apr 13, 2018

Conversation

MyonKeminta
Copy link
Contributor

On MVCC store, we encode keys to memcomparable format and append ts to it. However the DeleteRange method didn't encode it.
The new test case in mock_tikv_test.go reveals this problem, and now it's fixed.

commit e2c6be4
Author: MyonKeminta <mk@mkmkm.me>
Date:   Wed Apr 11 10:04:55 2018 +0800

    Fix: delete range in mocktikv didn't encode key

commit 88e8477
Merge: ef512dc e2269be
Author: MyonKeminta <mk@mkmkm.me>
Date:   Wed Apr 11 09:16:47 2018 +0800

    Merge branch 'master' into misono/mock-tikv-delete-range

commit ef512dc
Merge: 0a02583 13a7f19
Author: Lynn <zimu_xia@126.com>
Date:   Mon Apr 9 10:47:29 2018 +0800

    Merge branch 'master' into misono/mock-tikv-delete-range

commit 0a02583
Merge: 7ec31e8 fbae31e
Author: tiancaiamao <tiancaiamao@gmail.com>
Date:   Sun Apr 8 07:31:45 2018 -0500

    Merge branch 'master' into misono/mock-tikv-delete-range

commit 7ec31e8
Merge: 9cfc4e5 9e176ea
Author: Ewan Chou <coocood@gmail.com>
Date:   Wed Apr 4 22:24:35 2018 +0800

    Merge branch 'master' into misono/mock-tikv-delete-range

commit 9cfc4e5
Author: MyonKeminta <mk@mkmkm.me>
Date:   Wed Apr 4 00:24:18 2018 +0800

    Fix ts

commit e5da638
Author: MyonKeminta <mk@mkmkm.me>
Date:   Wed Apr 4 00:15:59 2018 +0800

    Add test

commit 1af5aea
Author: MyonKeminta <mk@mkmkm.me>
Date:   Tue Apr 3 22:23:20 2018 +0800

    Implement delete range in mvcc leveldb

commit bc1d6a1
Merge: c97fb30 ccf6da1
Author: MyonKeminta <mk@mkmkm.me>
Date:   Tue Apr 3 21:44:49 2018 +0800

    Merge branch 'master' into misono/mock-tikv-delete-range

commit c97fb30
Merge: f408426 c86cd59
Author: zhangjinpeng1987 <zhangjinpeng@pingcap.com>
Date:   Wed Mar 28 11:01:06 2018 +0800

    Merge branch 'master' into misono/mock-tikv-delete-range

commit f408426
Author: MyonKeminta <mk@mkmkm.me>
Date:   Mon Mar 26 15:00:01 2018 +0800

    store/mockstore: Add deleterange to mock tikv
@MyonKeminta
Copy link
Contributor Author

@tiancaiamao PTAL

}

return mvcc.db.Write(batch, nil)
return mvcc.doRawDeleteRange(codec.EncodeBytes(nil, startKey), codec.EncodeBytes(nil, endKey))
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 use mvccEncode?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't need to append timestamp here.

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Apr 12, 2018
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala
Copy link
Contributor

/run-all-tests

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Apr 12, 2018
@tiancaiamao
Copy link
Contributor

/run-all-tests

@tiancaiamao tiancaiamao merged commit 22ebc28 into pingcap:master Apr 13, 2018
@MyonKeminta MyonKeminta deleted the misono/fix-mock-delete-range branch April 13, 2018 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants