Skip to content
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

Add enable_remote_compaction option to DB Stress #13378

Closed

Conversation

jaykorean
Copy link
Contributor

Summary

First step to add (simulated) Remote Compaction in Stress Test. More PRs to come. Just first PR to add the FLAG to enable it. DbStressCompactionService will return kUseLocal for all compactions.

Test Plan

python3 -u tools/db_crashtest.py whitebox --enable_remote_compaction=1
python3 -u tools/db_crashtest.py blackbox --enable_remote_compaction=1

@jaykorean jaykorean requested a review from hx235 February 6, 2025 22:11
@jaykorean jaykorean changed the title Add enable_remote_compaction option to DB Stress - Part 1 Add enable_remote_compaction option to DB Stress Feb 6, 2025
@jaykorean jaykorean requested a review from cbi42 February 6, 2025 22:16
@jaykorean jaykorean marked this pull request as ready for review February 6, 2025 22:16
@facebook-github-bot
Copy link
Contributor

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

tools/db_crashtest.py Outdated Show resolved Hide resolved
tools/db_bench_tool.cc Outdated Show resolved Hide resolved
Copy link
Contributor

@hx235 hx235 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few nits - thanks

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

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

@facebook-github-bot
Copy link
Contributor

@jaykorean merged this pull request in 302254d.

@jaykorean jaykorean deleted the remote_compaction_in_stress_test branch February 7, 2025 01:07
facebook-github-bot pushed a commit that referenced this pull request Feb 12, 2025
Summary:
Options File Number to be read by remote worker is part of the `CompactionServiceInput`. We've been setting this in `ProcessKeyValueCompactionWithCompactionService()` while the db_mutex is not held. This needs to be accessed while the mutex is held. The value can change as part of `SetOptions() -> RenameTempFileToOptionsFile()` as in following.

https://github.com/facebook/rocksdb/blob/e6972196bca115e841a6b88d361ba945b49e1e5d/db/db_impl/db_impl.cc#L5595-L5596

Keep this value in memory during `CompactionJob::Prepare()` which is called while the mutex is held, so that we can easily access this later without mutex when building the CompactionInput for the remote compaction.

Thanks to the crash test. This was surfaced after #13378 merged.

Pull Request resolved: #13394

Test Plan:
Unit Test
```
./compaction_service_test
```

Crash Test
```
COERCE_CONTEXT_SWITCH=1 COMPILE_WITH_TSAN=1 CC=clang-13 CXX=clang++-13 ROCKSDB_DISABLE_ALIGNED_NEW=1 USE_CLANG=1 make V=1 -j100 dbg
```
```
python3 -u tools/db_crashtest.py blackbox --enable_remote_compaction=1
```

Reviewed By: jowlyzhang

Differential Revision: D69496313

Pulled By: jaykorean

fbshipit-source-id: 7e38e3cb75d5a7708beb4883e1a138e2b09ff837
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants