Provide support for subcompactions with user-defined timestamps #10344

akankshamahajan15 · 2022-07-12T02:48:44Z

Summary: The subcompaction logic currently picks file boundaries as subcompaction boundaries. This is not compatible with user-defined timestamps because of two issues.
Issue1: ReadOptions.iterate_lower_bound and ReadOptions.iterate_upper_bound contains timestamps which results in assertion failure as BlockBasedTableIterator expects bounds to be without timestamps. As result, because of wrong comparison end key is returned as user_key resulting in assertion failure.
Issue2: Since it might result in two keys that only differ by user timestamp getting processed by two different subcompactions (and thus two different CompactionIterator state machines), which in turn can cause data correction issues.

This PR provide support to reenable subcompactions with user-defined timestamps.

Test Plan: Added new unit test

Without fix for Issue1 unit test MultipleSubCompactions fails with error:

db_with_timestamp_compaction_test: ./db/compaction/clipping_iterator.h:247: void rocksdb::ClippingIterat│
or::AssertBounds(): Assertion `!valid_ || !end_ || cmp_->Compare(key(), *end_) < 0' failed.           
Received signal 6 (Aborted)                                                                             │
#0   /usr/local/fbcode/platform009/lib/libc.so.6(gsignal+0x100) [0x7f8fbbbfe530] db_with_timestamp_compaction_test: ./db/compaction/clipping_iterator.h:247: void rocksdb::ClippingIterator::AssertBounds(): Assertion `!valid_ || !end_ || cmp_->Compare(key(), *end_) < 0' failed. 
Aborted (core dumped)

Ran stress test
make crash_test_with_ts -j32

db/compaction/compaction.cc

facebook-github-bot · 2022-07-27T21:02:32Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-07-27T21:06:51Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-27T21:07:35Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-07-27T21:39:54Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-27T23:24:08Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot · 2022-07-28T01:51:13Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-28T19:57:19Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-28T20:01:28Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-28T20:11:52Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

riversand963

Thanks @akankshamahajan15 for the PR! A few minor comments.

It would be good if we can add a comment to ReadOptions::iterate_upper_bound and ReadOptions::iterate_lower_bound saying that they do not include user-defined timestamp.

db/compaction/compaction_job.cc

facebook-github-bot · 2022-07-29T17:35:03Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

riversand963

A few more minor comments. Otherwise, LGTM.
I don't think this PR has a risk of perf regression, but you can use db_bench waitforcompact benchmark and verify.

db/db_with_timestamp_compaction_test.cc

akankshamahajan15 · 2022-07-29T18:03:56Z

A few more minor comments. Otherwise, LGTM. I don't think this PR has a risk of perf regression, but you can use db_bench waitforcompact benchmark and verify.

For db_bench do I need to pass any other argument

 ./db_bench -db=/tmp/rocksdb_bench_test  -benchmarks="waitforcompaction"
Set seed to 1659117620245278 because --seed was 0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
Integrated BlobDB: blob cache disabled
RocksDB:    version 7.6.0
Date:       Fri Jul 29 11:00:20 2022
CPU:        32 * Intel Xeon Processor (Skylake)
CPUCache:   16384 KB
Keys:       16 bytes each (+ 0 bytes user-defined timestamp)
Values:     100 bytes each (50 bytes after compression)
Entries:    1000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    110.6 MB (estimated)
FileSize:   62.9 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
waitforcompaction(/tmp/rocksdb_bench_test): started
waitforcompaction(/tmp/rocksdb_bench_test): finished
waitforcompaction(/tmp/rocksdb_bench_test): started
waitforcompaction(/tmp/rocksdb_bench_test): finished

It doesn't print the time taken.

Summary: The subcompaction logic currently picks file boundaries as subcompaction boundaries. This is not compatible with user-defined timestamps, as ReadOptions.iterate_lower_bound and ReadOptions.iterate_upper_bound contains timestamps. Test Plan: Added new unit test

…boundaries.

facebook-github-bot · 2022-07-29T19:08:27Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-29T19:08:58Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

riversand963 · 2022-07-29T19:33:12Z

A few more minor comments. Otherwise, LGTM. I don't think this PR has a risk of perf regression, but you can use db_bench waitforcompact benchmark and verify.

For db_bench do I need to pass any other argument

 ./db_bench -db=/tmp/rocksdb_bench_test  -benchmarks="waitforcompaction"
Set seed to 1659117620245278 because --seed was 0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
Integrated BlobDB: blob cache disabled
RocksDB:    version 7.6.0
Date:       Fri Jul 29 11:00:20 2022
CPU:        32 * Intel Xeon Processor (Skylake)
CPUCache:   16384 KB
Keys:       16 bytes each (+ 0 bytes user-defined timestamp)
Values:     100 bytes each (50 bytes after compression)
Entries:    1000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    110.6 MB (estimated)
FileSize:   62.9 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
waitforcompaction(/tmp/rocksdb_bench_test): started
waitforcompaction(/tmp/rocksdb_bench_test): finished
waitforcompaction(/tmp/rocksdb_bench_test): started
waitforcompaction(/tmp/rocksdb_bench_test): finished

It doesn't print the time taken.

hmm, how about benchmark "compactall".

TEST_TMPDIR=/dev/shm/ ./db_bench -disable_auto_compactions=1 -benchmarks=fillrandom -threads=32
TEST_TMPDIR=/dev/shm/ -disable_auto_compactions=1 -use_existing_db=1 -benchmarks=compactall -subcompactions=3
``

Benchmark compactall does not report elapsed time either, but you can either add some code or simply use linux `time` command?

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

facebook-github-bot · 2022-07-29T20:09:02Z

@akankshamahajan15 has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot · 2022-07-29T20:15:03Z

@akankshamahajan15 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

akankshamahajan15 · 2022-07-31T18:30:26Z

Db_bench:
Time in Nanos
Without changes:

TEST_TMPDIR=/dev/shm/ ./db_bench -disable_auto_compactions=1 -benchmarks=fillrandom -threads=32
TEST_TMPDIR=/dev/shm/ ./db_bench --disable_auto_compactions=1 -use_existing_db=1 -benchmarks=compactall -subcompactions=3
Set seed to 1659290798535829 because --seed was 0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
Integrated BlobDB: blob cache disabled
RocksDB:    version 7.6.0
Date:       Sun Jul 31 11:06:40 2022
CPU:        32 * Intel Xeon Processor (Skylake)
CPUCache:   16384 KB
Keys:       16 bytes each (+ 0 bytes user-defined timestamp)
Values:     100 bytes each (50 bytes after compression)
Entries:    1000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    110.6 MB (estimated)
FileSize:   62.9 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
Time taken by CompactAll: 4632565417

With Changes:

TEST_TMPDIR=/dev/shm/ ./db_bench -disable_auto_compactions=1 -benchmarks=fillrandom -threads=32
TEST_TMPDIR=/dev/shm/ ./db_bench --disable_auto_compactions=1 -use_existing_db=1 -benchmarks=compactall -subcompactions=3
Set seed to 1659292183163276 because --seed was 0
Initializing RocksDB Options from the specified file
Initializing RocksDB Options from command-line flags
Integrated BlobDB: blob cache disabled
RocksDB:    version 7.6.0
Date:       Sun Jul 31 11:29:43 2022
CPU:        32 * Intel Xeon Processor (Skylake)
CPUCache:   16384 KB
Keys:       16 bytes each (+ 0 bytes user-defined timestamp)
Values:     100 bytes each (50 bytes after compression)
Entries:    1000000
Prefix:    0 bytes
Keys per prefix:    0
RawSize:    110.6 MB (estimated)
FileSize:   62.9 MB (estimated)
Write rate: 0 bytes/second
Read rate: 0 ops/second
Compression: Snappy
Compression sampling rate: 0
Memtablerep: SkipListFactory
Perf Level: 1
------------------------------------------------
Time taken by CompactAll: 4685684075

facebook-github-bot added the CLA Signed label Jul 12, 2022

akankshamahajan15 force-pushed the user_defined_timestamp branch from 3b291a6 to 75c7d69 Compare July 14, 2022 18:41

akankshamahajan15 force-pushed the user_defined_timestamp branch 2 times, most recently from 082b3b6 to f9c357a Compare July 27, 2022 20:51

akankshamahajan15 changed the title ~~[WIP] Support subcompactions with user-defined timestamps~~ Fix boundaries for subcompactions with user-defined timestamps Jul 27, 2022

akankshamahajan15 requested a review from riversand963 July 27, 2022 20:52

akankshamahajan15 commented Jul 27, 2022

View reviewed changes

db/compaction/compaction.cc Show resolved Hide resolved

akankshamahajan15 force-pushed the user_defined_timestamp branch from f9c357a to 594cb27 Compare July 27, 2022 21:06

akankshamahajan15 force-pushed the user_defined_timestamp branch from 594cb27 to 598a96f Compare July 27, 2022 21:39

akankshamahajan15 force-pushed the user_defined_timestamp branch from 598a96f to 56304f4 Compare July 28, 2022 01:51

akankshamahajan15 changed the title ~~Fix boundaries for subcompactions with user-defined timestamps~~ Provide support for subcompactions with user-defined timestamps Jul 28, 2022

akankshamahajan15 force-pushed the user_defined_timestamp branch from 56304f4 to bbe903c Compare July 28, 2022 19:57

akankshamahajan15 force-pushed the user_defined_timestamp branch from bbe903c to 67603af Compare July 28, 2022 20:01

riversand963 reviewed Jul 28, 2022

View reviewed changes

db/compaction/compaction_job.cc Show resolved Hide resolved

db/compaction/compaction_job.cc Outdated Show resolved Hide resolved

db/compaction/compaction_job.cc Outdated Show resolved Hide resolved

db/compaction/compaction_job.cc Outdated Show resolved Hide resolved

akankshamahajan15 force-pushed the user_defined_timestamp branch from 67603af to be51fdd Compare July 29, 2022 17:35

riversand963 approved these changes Jul 29, 2022

View reviewed changes

db/db_with_timestamp_compaction_test.cc Show resolved Hide resolved

db/db_with_timestamp_compaction_test.cc Outdated Show resolved Hide resolved

akankshamahajan15 added 2 commits July 29, 2022 12:08

Remove duplicate user keys (keys with different timestamps) from the …

8471413

…boundaries.

akankshamahajan15 force-pushed the user_defined_timestamp branch from be51fdd to 59086c7 Compare July 29, 2022 19:08

Addressed comments

c02d710

Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:

akankshamahajan15 force-pushed the user_defined_timestamp branch from 59086c7 to c02d710 Compare July 29, 2022 20:08

riversand963 approved these changes Jul 29, 2022

View reviewed changes

facebook-github-bot closed this in 56463d4 Jul 31, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide support for subcompactions with user-defined timestamps #10344

Provide support for subcompactions with user-defined timestamps #10344

akankshamahajan15 commented Jul 12, 2022 •

edited

Loading

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

riversand963 left a comment

facebook-github-bot commented Jul 29, 2022

riversand963 left a comment

akankshamahajan15 commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

riversand963 commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

akankshamahajan15 commented Jul 31, 2022 •

edited

Loading

Provide support for subcompactions with user-defined timestamps #10344

Provide support for subcompactions with user-defined timestamps #10344

Conversation

akankshamahajan15 commented Jul 12, 2022 • edited Loading

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 27, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

facebook-github-bot commented Jul 28, 2022

riversand963 left a comment

Choose a reason for hiding this comment

facebook-github-bot commented Jul 29, 2022

riversand963 left a comment

Choose a reason for hiding this comment

akankshamahajan15 commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

riversand963 commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

facebook-github-bot commented Jul 29, 2022

akankshamahajan15 commented Jul 31, 2022 • edited Loading

akankshamahajan15 commented Jul 12, 2022 •

edited

Loading

akankshamahajan15 commented Jul 31, 2022 •

edited

Loading