Skip to content

Commit

Permalink
Set Options File Number for CompactionInput under Mutex Lock (#13394)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jaykorean authored and facebook-github-bot committed Feb 12, 2025
1 parent e697219 commit a30c020
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 4 deletions.
2 changes: 2 additions & 0 deletions db/compaction/compaction_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,8 @@ void CompactionJob::Prepare(
// seqno threshold.)
penultimate_after_seqno_ = std::max(preclude_last_level_min_seqno,
c->GetKeepInLastLevelThroughSeqno());

options_file_number_ = versions_->options_file_number();
}

uint64_t CompactionJob::GetSubcompactionsLimit() {
Expand Down
4 changes: 4 additions & 0 deletions db/compaction/compaction_job.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,10 @@ class CompactionJob {
// the last level (output to penultimate level).
SequenceNumber penultimate_after_seqno_ = kMaxSequenceNumber;

// Options File Number used for Remote Compaction
// Setting this requires DBMutex.
uint64_t options_file_number_ = 0;

// Get table file name in where it's outputting to, which should also be in
// `output_directory_`.
virtual std::string GetTableFileName(uint64_t file_number);
Expand Down
5 changes: 1 addition & 4 deletions db/compaction/compaction_service_job.cc
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,7 @@ CompactionJob::ProcessKeyValueCompactionWithCompactionService(
compaction_input.has_end = sub_compact->end.has_value();
compaction_input.end =
compaction_input.has_end ? sub_compact->end->ToString() : "";
compaction_input.options_file_number =
sub_compact->compaction->input_version()
->version_set()
->options_file_number();
compaction_input.options_file_number = options_file_number_;

TEST_SYNC_POINT_CALLBACK(
"CompactionServiceJob::ProcessKeyValueCompactionWithCompactionService",
Expand Down

0 comments on commit a30c020

Please sign in to comment.