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

txn:improvement for point get by unique key or pk #1608

Merged
merged 7 commits into from
Feb 20, 2017

Conversation

AndreMouche
Copy link
Member

@AndreMouche AndreMouche commented Feb 13, 2017

This PR implement improvement for point get by unique key or pk with tidb.
For Get with ts==uint64::max(which means it's a point get by pk or unique_key from tidb), if lock exist and current key the primary, return latest committed version instead of ErrorWithLock.
@disksing @zhangjinpeng1987 PTAL

…ts==uint64::max(which means is a point get by pk or unique_key in tidb)
@siddontang
Copy link
Contributor

PTAL @hhkbp2

ts: lock.ts,
ttl: lock.ttl,
});
let raw_key = try!(key.raw());
Copy link
Contributor

Choose a reason for hiding this comment

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

Move it into if ts == u64::MAX.

if ts == u64::MAX && raw_key == primary_key {
// when ts==MAX(which means it's a point get by pk or unique key),
// and current key is the primary key,
// return latest commit version's value
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks strange for MVCC to be mixed with the concept of pk or unique key. Maybe better to extract a get_latest(&mut self, key: &Key) method.

Copy link
Member Author

Choose a reason for hiding this comment

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

I do not think is a good idea to extract get_latest(&mut self, key: &Key) since we use this through coprocessor, use get_latest may change too much.

@ngaut
Copy link
Member

ngaut commented Feb 17, 2017

PTAL

ttl: lock.ttl,
});
if ts == u64::MAX && try!(key.raw()) == lock.primary {
// when ts==MAX(which means get latest committed version for primary key),
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ts== MAX(which means get / ts == u64::MAX (which means to get/

@hhkbp2
Copy link
Contributor

hhkbp2 commented Feb 17, 2017

Rest LGTM

@disksing
Copy link
Contributor

LGTM

@ngaut
Copy link
Member

ngaut commented Feb 18, 2017

PTAL @hhkbp2

@siddontang
Copy link
Contributor

any performance increased?

Copy link
Contributor

@siddontang siddontang left a comment

Choose a reason for hiding this comment

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

LGTM

@siddontang siddontang merged commit 6b06d53 into master Feb 20, 2017
@ngaut ngaut deleted the shirly/point_get_improve branch March 12, 2017 22:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants