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

use delete range to destroy a region #1988

Merged
merged 47 commits into from Aug 30, 2017

Conversation

Projects
None yet
5 participants
@zhangjinpeng1987
Member

zhangjinpeng1987 commented Jul 3, 2017

No description provided.

@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987

zhangjinpeng1987 Jul 17, 2017

Member

Should wait this release

Member

zhangjinpeng1987 commented Jul 17, 2017

Should wait this release

@zhangjinpeng1987 zhangjinpeng1987 changed the title from [DNM] use delete range to destroy a region to use delete range to destroy a region Jul 26, 2017

zhangjinpeng1987 added some commits Jul 26, 2017

fmt
@@ -83,6 +87,39 @@ pub fn conf_change_type_str(conf_type: &eraftpb::ConfChangeType) -> &'static str
}
}
// Use delete range to delete all data in [start_key, end_key) for each column family.
pub fn delete_all_in_range(db: &DB, start_key: &[u8], end_key: &[u8]) -> Result<()> {

This comment has been minimized.

@siddontang

siddontang Jul 26, 2017

Contributor

please add a test for this

@siddontang

siddontang Jul 26, 2017

Contributor

please add a test for this

@zhangjinpeng1987 zhangjinpeng1987 requested review from BusyJay and choleraehyq Jul 27, 2017

@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987
Member

zhangjinpeng1987 commented Jul 31, 2017

@BusyJay PTAL

Cholerae Hu and others added some commits Aug 3, 2017

Cholerae Hu
util: test for prefix bloom
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>

@siddontang siddontang changed the title from use delete range to destroy a region to [DNM] use delete range to destroy a region Aug 9, 2017

@siddontang siddontang added this to the 0ohtjj9o0i8tj5g587ik9ofgnr66gfrrf6tgrt6ujjbuju j67b uuhj6uh5 thuuuiuuiui.8988ui89kkll-=p;lpp;p[;;/p;[;['/""" milestone Aug 9, 2017

zhangjinpeng1987 added some commits Aug 14, 2017

@lishuai87

This comment has been minimized.

Show comment
Hide comment
@lishuai87

lishuai87 Aug 30, 2017

Contributor

clear_meta can also use delete range

Contributor

lishuai87 commented Aug 30, 2017

clear_meta can also use delete range

@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987

zhangjinpeng1987 Aug 30, 2017

Member

@javaforfun I don't think it is worthy to use delete range here, in most cases there is not too much meta datas should be cleared.

Member

zhangjinpeng1987 commented Aug 30, 2017

@javaforfun I don't think it is worthy to use delete range here, in most cases there is not too much meta datas should be cleared.

@lishuai87

This comment has been minimized.

Show comment
Hide comment
@lishuai87

lishuai87 Aug 30, 2017

Contributor

LGTM

Contributor

lishuai87 commented Aug 30, 2017

LGTM

@zhangjinpeng1987 zhangjinpeng1987 changed the title from [DNM] use delete range to destroy a region to use delete range to destroy a region Aug 30, 2017

@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987
Member

zhangjinpeng1987 commented Aug 30, 2017

@BusyJay PTAL

// function prefix_extractor->Transform, in our case the prefix_extractor is
// FixedSuffixSliceTransform, if the length of start key less than 8, we
// will encounter index out of range error.
box_try!(wb.delete_range_cf(handle, it.key(), end_key));

This comment has been minimized.

@BusyJay

BusyJay Aug 30, 2017

Contributor

Is it possible to let bloom filter be compatible with this?

@BusyJay

BusyJay Aug 30, 2017

Contributor

Is it possible to let bloom filter be compatible with this?

This comment has been minimized.

@BusyJay

BusyJay Aug 30, 2017

Contributor

What happen if it.key() >= end_key?

@BusyJay

BusyJay Aug 30, 2017

Contributor

What happen if it.key() >= end_key?

This comment has been minimized.

@zhangjinpeng1987

zhangjinpeng1987 Aug 30, 2017

Member

it.key must > end_key, because end_key is upper_bound when we create the iterator

@zhangjinpeng1987

zhangjinpeng1987 Aug 30, 2017

Member

it.key must > end_key, because end_key is upper_bound when we create the iterator

zhangjinpeng1987 added some commits Aug 30, 2017

@ngaut

This comment has been minimized.

Show comment
Hide comment
@ngaut

ngaut Aug 30, 2017

Member

Should we return error?

Member

ngaut commented on src/util/rocksdb/mod.rs in 08ddba4 Aug 30, 2017

Should we return error?

zhangjinpeng1987 added some commits Aug 30, 2017

@BusyJay

This comment has been minimized.

Show comment
Hide comment
@BusyJay

BusyJay Aug 30, 2017

Contributor

LGTM

Contributor

BusyJay commented Aug 30, 2017

LGTM

@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987
Member

zhangjinpeng1987 commented Aug 30, 2017

@zhangjinpeng1987 zhangjinpeng1987 merged commit 89f80d1 into master Aug 30, 2017

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
license/cla Contributor License Agreement is signed.
Details

@zhangjinpeng1987 zhangjinpeng1987 deleted the zhangjinpeng/delete-range branch Aug 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment