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/tikv: implement MVCCStore interface using leveldb backend #3970

Merged
merged 22 commits into from Aug 22, 2017

Conversation

Projects
None yet
5 participants
@tiancaiamao
Contributor

tiancaiamao commented Aug 1, 2017

This PR intent to persistent store for mocktikv, some necessary interface are implemented.

PTAL @disksing @hanfei1991

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Aug 7, 2017

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Aug 8, 2017

Contributor

PTAL @disksing

Contributor

tiancaiamao commented Aug 8, 2017

PTAL @disksing

@disksing

This comment has been minimized.

Show comment
Hide comment
@disksing

disksing Aug 8, 2017

Member

LGTM.

Member

disksing commented Aug 8, 2017

LGTM.

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Aug 9, 2017

@zz-jason zz-jason added the status/LGT1 label Aug 9, 2017

Show outdated Hide outdated store/tikv/mock-tikv/mvcc.go
Show outdated Hide outdated store/tikv/mock-tikv/mvcc.go
Show outdated Hide outdated store/tikv/mock-tikv/mvcc.go
Show outdated Hide outdated store/tikv/mock-tikv/mvcc.go
@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Aug 11, 2017

Show outdated Hide outdated store/tikv/mock-tikv/rpc.go
mu sync.RWMutex
}
var lockVer uint64 = math.MaxUint64

This comment has been minimized.

@zimulala

zimulala Aug 11, 2017

Member

Why the value of lockVer is math.MaxUint64?

@zimulala

zimulala Aug 11, 2017

Member

Why the value of lockVer is math.MaxUint64?

This comment has been minimized.

@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

because I want this key layout:

	// Key layout:
	// ...
	// Key_lock        -- (0)
	// Key_verMax      -- (1)
	// ...
	// Key_ver+1       -- (2)
	// Key_ver         -- (3)
	// Key_ver-1       -- (4)
	// ...
	// Key_0           -- (5)
	// NextKey_verMax  -- (6)
	// ...
	// NextKey_ver+1   -- (7)
	// NextKey_ver     -- (8)
	// NextKey_ver-1   -- (9)
	// ...
	// NextKey_0       -- (10)
	// ...
	// EOF
@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

because I want this key layout:

	// Key layout:
	// ...
	// Key_lock        -- (0)
	// Key_verMax      -- (1)
	// ...
	// Key_ver+1       -- (2)
	// Key_ver         -- (3)
	// Key_ver-1       -- (4)
	// ...
	// Key_0           -- (5)
	// NextKey_verMax  -- (6)
	// ...
	// NextKey_ver+1   -- (7)
	// NextKey_ver     -- (8)
	// NextKey_ver-1   -- (9)
	// ...
	// NextKey_0       -- (10)
	// ...
	// EOF
Show outdated Hide outdated store/tikv/mock-tikv/mvcc_leveldb.go
// mvccEncode returns the encoded key.
func mvccEncode(key []byte, ver uint64) []byte {
b := codec.EncodeBytes(nil, key)
ret := codec.EncodeUintDesc(b, ver)

This comment has been minimized.

@zimulala

zimulala Aug 11, 2017

Member

Why use Desc?

@zimulala

zimulala Aug 11, 2017

Member

Why use Desc?

This comment has been minimized.

@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

ditto

@tiancaiamao

tiancaiamao Aug 11, 2017

Contributor

ditto

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao
Contributor

tiancaiamao commented Aug 11, 2017

@hanfei1991

This comment has been minimized.

Show comment
Hide comment
@hanfei1991

hanfei1991 Aug 11, 2017

Member

We can use MVCCLevelDB as default storage in this pr @tiancaiamao

Member

hanfei1991 commented Aug 11, 2017

We can use MVCCLevelDB as default storage in this pr @tiancaiamao

Show outdated Hide outdated store/tikv/mock-tikv/mvcc_leveldb.go
Show outdated Hide outdated store/tikv/mock-tikv/mvcc_leveldb.go
}
// ReverseScan implements the MVCCStore interface.
func (mvcc *MVCCLevelDB) ReverseScan(startKey, endKey []byte, limit int, startTS uint64, isoLevel kvrpcpb.IsolationLevel) []Pair {

This comment has been minimized.

@zimulala

zimulala Aug 14, 2017

Member

Add a TODO?

@zimulala

zimulala Aug 14, 2017

Member

Add a TODO?

Show outdated Hide outdated store/tikv/mock-tikv/mvcc_leveldb.go

tiancaiamao added some commits Aug 15, 2017

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Aug 15, 2017

Contributor

Now I've make TiDB binary use mocktikv by default.
PTAL @hanfei1991 @zimulala

Contributor

tiancaiamao commented Aug 15, 2017

Now I've make TiDB binary use mocktikv by default.
PTAL @hanfei1991 @zimulala

fix bug...
prewrite meet a rollback value whose commit ts larger is a write conflict!
Show outdated Hide outdated store/tikv/kv.go
Show outdated Hide outdated store/tikv/mock-tikv/mvcc_leveldb.go
if err1 != nil {
return errors.Trace(err1)
}
if ok && c.valueType != typeRollback {

This comment has been minimized.

@zimulala

zimulala Aug 22, 2017

Member

Please add some comments.

@zimulala

zimulala Aug 22, 2017

Member

Please add some comments.

// Key_ver -- (3)
// Key_ver-1 -- (4)
// ...
// Key_0 -- (5)

This comment has been minimized.

@zimulala

zimulala Aug 22, 2017

Member

Do we need to add a lock here?

@zimulala

zimulala Aug 22, 2017

Member

Do we need to add a lock here?

This comment has been minimized.

@tiancaiamao

tiancaiamao Aug 22, 2017

Contributor

Done

@tiancaiamao

tiancaiamao Aug 22, 2017

Contributor

Done

@@ -300,6 +434,12 @@ type RawKV interface {
RawDelete(key []byte)
}
// MVCCDebugger is for debugging.
type MVCCDebugger interface {

This comment has been minimized.

@zimulala

zimulala Aug 22, 2017

Member

Why remove these functions out of MVCCStore?

@zimulala

zimulala Aug 22, 2017

Member

Why remove these functions out of MVCCStore?

This comment has been minimized.

@tiancaiamao

tiancaiamao Aug 22, 2017

Contributor

A interface should be simple...The more methods a interface contain, the harder to implement the interface.
So I remove some non-essential methods from MVCCStore to MVCCDebugger, those methods are only necessary for debugging purpose.

@tiancaiamao

tiancaiamao Aug 22, 2017

Contributor

A interface should be simple...The more methods a interface contain, the harder to implement the interface.
So I remove some non-essential methods from MVCCStore to MVCCDebugger, those methods are only necessary for debugging purpose.

This comment has been minimized.

@zimulala

zimulala Aug 22, 2017

Member

Gotcha.

@zimulala

zimulala Aug 22, 2017

Member

Gotcha.

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Aug 22, 2017

Member

Do we need to add some tests for mvcc_leveldb.go?

Member

zimulala commented Aug 22, 2017

Do we need to add some tests for mvcc_leveldb.go?

@tiancaiamao

This comment has been minimized.

Show comment
Hide comment
@tiancaiamao

tiancaiamao Aug 22, 2017

Contributor

No, it's already covered.
MVCCLevelDB and MvccStore both implement MVCCStore interface, and that interface will be tested in mock_tikv_test.go @zimulala

Contributor

tiancaiamao commented Aug 22, 2017

No, it's already covered.
MVCCLevelDB and MvccStore both implement MVCCStore interface, and that interface will be tested in mock_tikv_test.go @zimulala

@zimulala

This comment has been minimized.

Show comment
Hide comment
@zimulala

zimulala Aug 22, 2017

Member

LGTM

Member

zimulala commented Aug 22, 2017

LGTM

@zimulala zimulala added status/LGT2 and removed status/LGT1 labels Aug 22, 2017

@tiancaiamao tiancaiamao merged commit ff34a46 into master Aug 22, 2017

2 of 3 checks passed

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

@tiancaiamao tiancaiamao deleted the tiancaiamao/mvcc-leveldb branch Aug 22, 2017

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