tikv / agatedb

A persistent key-value storage in rust.
Apache License 2.0
829 stars 74 forks source link

max_version support #190

Closed wangnengjie closed 2 years ago

wangnengjie commented 2 years ago

Signed-off-by: wangnengjie 751614701@qq.com

Support max_version and initialize next_txn_ts when open.

In this PR, the max_version is the max version in memtable, immutable memtable and all SST, means the max version that has been written to database.

The max_version is some what like lsn in rocksdb but not equivalent. In agatedb, we get (or user set) commit_ts from oracle when commiting transaction and the commit_ts is set as version of entries in this transaction.

When reopening (or recovering from) existed db, we need max_version to initializenext_txn_ts because the read_ts is next_txn_ts - 1 (non manage mode). Just like init lsn in rocksdb. Otherwise, we get read_ts that is 0 after reopen and no exist data can be seen. Moreover, as next_txn_ts will increase from 1 again, old data might be overwritten with same version and some data can magically 'appear' and 'disappear' with next_txn_ts increasing. Without max_version, agatedb is only a one-off database.

wangnengjie commented 2 years ago

LGTM, how about adding unit tests for MemTables?

https://github.com/tikv/agatedb/blob/master/src/memtable.rs#L94-L100

seems that memtable now assumes version of adding keys in monotone increase mode. Is that as expected?

GanZiheng commented 2 years ago

LGTM, how about adding unit tests for MemTables?

https://github.com/tikv/agatedb/blob/master/src/memtable.rs#L94-L100

seems that memtable now assumes version of adding keys in monotone increase mode. Is that as expected?

I think that is expected. Because we will call memtable::MemTable::put when writing updates to the LSM tree, and when using transaction with write_ch_lock, it is guaranteed that the timestamp of the updates pushed to the write channel is monotonically increasing.

But I think we may add comment for this or also update the max_version by comparing.

codecov[bot] commented 2 years ago

Codecov Report

Merging #190 (bfc07a2) into master (d8fbdef) will increase coverage by 0.19%. The diff coverage is 100.00%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #190 +/- ## ========================================== + Coverage 89.55% 89.74% +0.19% ========================================== Files 39 39 Lines 8752 8838 +86 ========================================== + Hits 7838 7932 +94 + Misses 914 906 -8 ```
wangnengjie commented 2 years ago

LGTM, how about adding unit tests for MemTables?

https://github.com/tikv/agatedb/blob/master/src/memtable.rs#L94-L100

seems that memtable now assumes version of adding keys in monotone increase mode. Is that as expected?

I think that is expected. Because we will call memtable::MemTable::put when writing updates to the LSM tree, and when using transaction with write_ch_lock, it is guaranteed that the timestamp of the updates pushed to the write channel is monotonically increasing.

But I think we may add comment for this or also update the max_version by comparing.

For managed_db mode, commit_ts is set by user. It might not be guaranteed?

GanZiheng commented 2 years ago

LGTM, how about adding unit tests for MemTables?

https://github.com/tikv/agatedb/blob/master/src/memtable.rs#L94-L100 seems that memtable now assumes version of adding keys in monotone increase mode. Is that as expected?

I think that is expected. Because we will call memtable::MemTable::put when writing updates to the LSM tree, and when using transaction with write_ch_lock, it is guaranteed that the timestamp of the updates pushed to the write channel is monotonically increasing. But I think we may add comment for this or also update the max_version by comparing.

For managed_db mode, commit_ts is set by user. It might not be guaranteed?

Yes, so I think it would be better to update by comparing.

wangnengjie commented 2 years ago

Done

GanZiheng commented 2 years ago

PTAL @Connor1996.

BusyJay commented 2 years ago

It will be helpful if the description of the PR or a linked issue can explain what is max_version.

wangnengjie commented 2 years ago

It will be helpful if the description of the PR or a linked issue can explain what is max_version.

Done. If somewhere is still puzzling, plz point out.