Enable backward iterator for keys with user-defined timestamp #8035
Conversation
riversand963
force-pushed
the
db-iter-ts-rev
branch
from
bd3b932
to
ec90dea
Compare
db/db_iter.cc
Outdated
| const std::string kTsMin(timestamp_size_, | ||
| static_cast<unsigned char>(0)); |
There was a problem hiding this comment.
std::string is actually std::basic_string<char>, so I actually think the original type was correct. We could consider simply using '\0' though instead of static_casting.
There was a problem hiding this comment.
I can try that. Cast to unsigned char due to a warning from VS.
|
Thanks @ltamasi for the review. |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
db/db_iter.cc
Outdated
| kValueTypeForSeek); | ||
| if (timestamp_size_ > 0) { | ||
| // TODO: pre-create kTsMax. | ||
| const std::string kTsMax(timestamp_size_, static_cast<char>(0xffu)); |
Also run db_bench For seekrandom, warmup 1 run, and test 3 runs. ``` ./db_bench -db=/dev/shm/rocksdb -disable_wal=1 -benchmarks=fillseq,seekrandom[-W1-X3] -reverse_iterator=1 -seek_nexts=5 ```
…f regression Rerun tests: ``` ./db_bench -db=/dev/shm/rocksdb -disable_wal=1 -benchmarks=fillseq,seekrandom[-W1-X6] -reverse_iterator=1 -seek_nexts=5 ``` master: ``` seekrandom [AVG 6 runs] : 96115 ops/sec; 53.2 MB/sec seekrandom [MEDIAN 6 runs] : 98075 ops/sec; 54.2 MB/sec ``` This PR: ``` seekrandom [AVG 6 runs] : 95521 ops/sec; 52.8 MB/sec seekrandom [MEDIAN 6 runs] : 96338 ops/sec; 53.3 MB/sec ``` These tests show that newly-added timestamp-related code does not add overhead to existing backward iteration code without timestamp. The addition of EqualWithoutTimestamp() to BytewiseComparator is crucial.
Also run db_bench ``` ./db_bench -user_timestamp_size=8 -db=/dev/shm/rocksdb -disable_wal=1 -benchmarks=fillseq,seekrandom[-W1-X6] -reverse_iterator=1 -seek_nexts=5 ``` Result ``` seekrandom [AVG 6 runs] : 90514 ops/sec; 50.1 MB/sec seekrandom [MEDIAN 6 runs] : 90834 ops/sec; 50.2 MB/sec ```
riversand963
force-pushed
the
db-iter-ts-rev
branch
from
4eee2bd
to
115a414
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
There was a problem hiding this comment.
@riversand963 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@riversand963 merged this pull request in 82b3888. |
This PR does the following:
Comparator::EqualWithoutTimestamp().SetTimestamp().Run db_bench (built with DEBUG_LEVEL=0) to demonstrate that no overhead is introduced for CPU-intensive workloads with a lot of
Prev(). Also provided results of iterating keys with timestamps.Results:
Result: