Fix a bug by setting up subcompaction bounds properly #10658
Conversation
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
riversand963
force-pushed
the
fix-subcompact-range
branch
from
72bc29c
to
50ab541
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
2 similar comments
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@akankshamahajan15 I am working on adding unit tests, but the fix is ready for review. Thanks |
| if (start.has_value()) { | ||
| start_ikey.SetInternalKey(start.value(), kMaxSequenceNumber, | ||
| kValueTypeForSeek); | ||
| if (ts_sz > 0) { | ||
| start_ikey.UpdateInternalKey(kMaxSequenceNumber, kValueTypeForSeek, |
There was a problem hiding this comment.
IIUC, both the boundary keys (start and end) need to have maximum timestamp?
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
1 similar comment
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
When user-defined timestamp is enabled, subcompaction bounds should be set up properly. When creating InputIterator for the compaction, the `start` and `end` should have their timestamp portions set to kMaxTimestamp, which is the highest possible timestamp. This is similar to what we do with setting up their sequence numbers to `kMaxSequenceNumber`. (Will add new unit tests) Test plan: ```bash make check rm -rf /dev/shm/rocksdb/* && mkdir /dev/shm/rocksdb/rocksdb_crashtest_expected && ./db_stress --allow_data_in_errors=True --clear_column_family_one_in=0 --continuous_verification_interval=0 --data_block_index_type=1 --db=/dev/shm/rocksdb//rocksdb_crashtest_blackbox --delpercent=5 --delrangepercent=0 --expected_values_dir=/dev/shm/rocksdb//rocksdb_crashtest_expected --iterpercent=0 --max_background_compactions=20 --max_bytes_for_level_base=10485760 --max_key=25000000 --max_write_batch_group_size_bytes=1048576 --nooverwritepercent=1 --ops_per_thread=300000 --paranoid_file_checks=1 --partition_filters=0 --prefix_size=8 --prefixpercent=5 --readpercent=30 --reopen=0 --snapshot_hold_ops=100000 --subcompactions=4 --target_file_size_base=65536 --target_file_size_multiplier=2 --test_batches_snapshots=0 --test_cf_consistency=0 --use_multiget=1 --user_timestamp_size=8 --value_size_mult=32 --verify_checksum=1 --write_buffer_size=65536 --writepercent=60 -disable_wal=1 -column_families=1 ```
…start from level 0 And this has happened to work for our existing use cases
riversand963
force-pushed
the
fix-subcompact-range
branch
from
6fa3c5b
to
f707c72
Compare
|
@riversand963 has updated the pull request. You must reimport the pull request before landing. |
|
@riversand963 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Summary: Hopefully, we can re-enable the combination of user-defined timestamp and subcompactions after #10658. Pull Request resolved: #10689 Test Plan: Make sure the following succeeds on devserver. make crash_test_with_ts Reviewed By: ltamasi Differential Revision: D39556558 Pulled By: riversand963 fbshipit-source-id: 4695f420b1bc9ebf3b24640b693746f4db82c149
When user-defined timestamp is enabled, subcompaction bounds should be set up properly. When creating InputIterator for the compaction, the
startandendshould have their timestamp portions set to kMaxTimestamp, which is the highest possible timestamp. This is similar to what we do with setting up their sequence numbers tokMaxSequenceNumber.(Will add new unit tests)
Test plan: