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

tikv-ctl: improve scan and fix a bug for mvcc #2964

Merged
merged 18 commits into from Apr 25, 2018

Conversation

Projects
None yet
5 participants
@hicqu
Copy link
Contributor

commented Apr 17, 2018

Use tikv-ctl scan with bad arguments maybe cause TiKV crash. This PR fix this.

@hicqu hicqu requested review from BusyJay and overvenus Apr 17, 2018

hicqu added some commits Apr 17, 2018

.short("f")
.long("from")
.takes_value(true)
.help("set the scan from raw key, in escaped format"),
)
.arg(
Arg::with_name("to")
.conflicts_with("limit")

This comment has been minimized.

Copy link
@siddontang

siddontang Apr 18, 2018

Contributor

why limit conflicts with to? I think getting limited items in [from, to) is reasonable.

@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2018

PTAL @overvenus .

@iamxy

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2018

/run-all-tests

@iamxy

This comment has been minimized.

Copy link
Collaborator

commented Apr 18, 2018

/rebuild

hicqu added some commits Apr 19, 2018

@@ -778,7 +791,7 @@ fn main() {
.conflicts_with_all(&["region", "index"])
.short("k")
.takes_value(true)
.help("set the raw key, in escaped form"),
.help("set the raw key (generally starts with \"z\"), in escaped form"),

This comment has been minimized.

Copy link
@overvenus

overvenus Apr 19, 2018

Contributor

How about replacing with a constant string?

@hicqu

This comment has been minimized.

Copy link
Contributor Author

commented Apr 20, 2018

hicqu added some commits Apr 23, 2018

@@ -855,8 +871,8 @@ fn main() {
.help("set the scan commit_ts as filter"),
)
.arg(
Arg::with_name("cf")
.long("cf")
Arg::with_name("show-cf")

This comment has been minimized.

Copy link
@breeswish

breeswish Apr 24, 2018

Member

why is this arg renamed?

This comment has been minimized.

Copy link
@hicqu

hicqu Apr 24, 2018

Author Contributor

cf is unclear. People will think the command scan on cfs they specified. But the command scan on write cf and print specified cfs.

This comment has been minimized.

Copy link
@breeswish

hicqu added some commits Apr 24, 2018

@breeswish

This comment has been minimized.

Copy link
Member

commented Apr 24, 2018

LGTM

@hicqu hicqu changed the title tikv-ctl: check arguments for mvcc_scan. tikv-ctl: improve scan and fix a bug for mvcc Apr 25, 2018

@hicqu hicqu merged commit 0809d52 into tikv:master Apr 25, 2018

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
jenkins-ci-tikv/build Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

@hicqu hicqu deleted the hicqu:tikv-ctl-fix branch Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.