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

snapshot refactor 2: move v2 api to super module #2057

Merged
merged 4 commits into from Jul 20, 2017

Conversation

Projects
None yet
4 participants

Cholerae Hu added some commits Jul 13, 2017

Cholerae Hu
snap: move v2 snap out of mod v2
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
Cholerae Hu
snap: remove warning
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
Show outdated Hide outdated src/raftstore/store/snap.rs
@@ -165,8 +170,8 @@ pub trait Snapshot: Read + Write + Send {
// Only used in tests.
pub fn copy_snapshot(mut from: Box<Snapshot>, mut to: Box<Snapshot>) -> io::Result<()> {
if !to.exists() {
try!(io::copy(&mut from, &mut to));
try!(to.save());
io::copy(&mut from, &mut to)?;

This comment has been minimized.

@siddontang

siddontang Jul 19, 2017

Contributor

don't use ? now.

@siddontang

siddontang Jul 19, 2017

Contributor

don't use ? now.

This comment has been minimized.

@andelf

andelf Jul 19, 2017

Contributor
src/bin/tikv-ctl.rs:426:            let (region_id, suffix) = keys::decode_region_meta_key(key)?;
src/raftstore/store/snap.rs:999:                    let handle = snap.cf_handle(cf_file.cf)?;
@andelf

andelf Jul 19, 2017

Contributor
src/bin/tikv-ctl.rs:426:            let (region_id, suffix) = keys::decode_region_meta_key(key)?;
src/raftstore/store/snap.rs:999:                    let handle = snap.cf_handle(cf_file.cf)?;

This comment has been minimized.

@choleraehyq

choleraehyq Jul 19, 2017

Seems all ? were written by me. But no one told me this directly before. Because of performance issue?

@choleraehyq

choleraehyq Jul 19, 2017

Seems all ? were written by me. But no one told me this directly before. Because of performance issue?

This comment has been minimized.

This comment has been minimized.

@siddontang

siddontang Jul 19, 2017

Contributor

I see that you have already introduced ?, please file a PR to remove all later.

@siddontang

siddontang Jul 19, 2017

Contributor

I see that you have already introduced ?, please file a PR to remove all later.

Cholerae Hu
snap: remove useless import path
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
@zhangjinpeng1987

zhangjinpeng1987 Jul 19, 2017

Member

rest LGTM

Member

zhangjinpeng1987 commented Jul 19, 2017

rest LGTM

Cholerae Hu
snap: remove all ?
Signed-off-by: Cholerae Hu <huyingqian@pingcap.com>
@zhangjinpeng1987

This comment has been minimized.

Show comment
Hide comment
Member

zhangjinpeng1987 commented Jul 19, 2017

LGTM

@choleraehyq

This comment has been minimized.

Show comment
Hide comment
if snap_keys.is_empty() {
return Ok(());
}
snap_keys.sort();

This comment has been minimized.

@siddontang

siddontang Jul 20, 2017

Contributor

why remove sort here?

@siddontang

siddontang Jul 20, 2017

Contributor

why remove sort here?

This comment has been minimized.

@choleraehyq

choleraehyq Jul 20, 2017

in list_idle_snap keys are already sorted.

@choleraehyq

choleraehyq Jul 20, 2017

in list_idle_snap keys are already sorted.

@siddontang

This comment has been minimized.

Show comment
Hide comment
@siddontang

siddontang Jul 20, 2017

Contributor

LGTM

Contributor

siddontang commented Jul 20, 2017

LGTM

@choleraehyq choleraehyq merged commit ad5af3f into pre-ga Jul 20, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla Contributor License Agreement is signed.
Details

@choleraehyq choleraehyq deleted the hyq/movev2 branch Jul 20, 2017

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