diff --git a/db/version_set.cc b/db/version_set.cc index 33d6ce495e..86f47aca73 100644 --- a/db/version_set.cc +++ b/db/version_set.cc @@ -5508,6 +5508,7 @@ Status VersionSet::ProcessManifestWrites( Status s; IOStatus io_s; IOStatus manifest_io_status; + std::unique_ptr new_desc_log_ptr; { FileOptions opt_file_opts = fs_->OptimizeForManifestWrite(file_options_); mu->Unlock(); @@ -5536,6 +5537,7 @@ Status VersionSet::ProcessManifestWrites( } } + log::Writer* raw_desc_log_ptr = descriptor_log_.get(); if (s.ok() && new_descriptor_log) { // This is fine because everything inside of this block is serialized -- // only one thread can be here at the same time @@ -5557,11 +5559,11 @@ Status VersionSet::ProcessManifestWrites( db_options_->listeners, nullptr, tmp_set.Contains(FileType::kDescriptorFile), tmp_set.Contains(FileType::kDescriptorFile))); - descriptor_log_.reset( + new_desc_log_ptr.reset( new log::Writer(std::move(file_writer), 0, false)); + raw_desc_log_ptr = new_desc_log_ptr.get(); s = WriteCurrentStateToManifest(write_options, curr_state, - wal_additions, descriptor_log_.get(), - io_s); + wal_additions, raw_desc_log_ptr, io_s); assert(s == io_s); } if (!io_s.ok()) { @@ -5607,7 +5609,7 @@ Status VersionSet::ProcessManifestWrites( } ++idx; #endif /* !NDEBUG */ - io_s = descriptor_log_->AddRecord(write_options, record); + io_s = raw_desc_log_ptr->AddRecord(write_options, record); if (!io_s.ok()) { s = io_s; manifest_io_status = io_s; @@ -5617,7 +5619,7 @@ Status VersionSet::ProcessManifestWrites( if (s.ok()) { io_s = - SyncManifest(db_options_, write_options, descriptor_log_->file()); + SyncManifest(db_options_, write_options, raw_desc_log_ptr->file()); manifest_io_status = io_s; TEST_SYNC_POINT_CALLBACK( "VersionSet::ProcessManifestWrites:AfterSyncManifest", &io_s); @@ -5650,7 +5652,7 @@ Status VersionSet::ProcessManifestWrites( if (s.ok()) { // find offset in manifest file where this version is stored. - new_manifest_file_size = descriptor_log_->file()->GetFileSize(); + new_manifest_file_size = raw_desc_log_ptr->file()->GetFileSize(); } if (first_writer.edit_list.front()->IsColumnFamilyDrop()) { @@ -5696,6 +5698,7 @@ Status VersionSet::ProcessManifestWrites( // Append the old manifest file to the obsolete_manifest_ list to be deleted // by PurgeObsoleteFiles later. if (s.ok() && new_descriptor_log) { + descriptor_log_ = std::move(new_desc_log_ptr); obsolete_manifests_.emplace_back( DescriptorFileName("", manifest_file_number_)); } @@ -5773,6 +5776,7 @@ Status VersionSet::ProcessManifestWrites( // corrupted. So we need to force the next version update to start a // new manifest file. descriptor_log_.reset(); + new_desc_log_ptr.reset(); // If manifest operations failed, then we know the CURRENT file still // points to the original MANIFEST. Therefore, we can safely delete the // new MANIFEST. diff --git a/unreleased_history/bug_fixes/manifest_recovery_ingest_race.md b/unreleased_history/bug_fixes/manifest_recovery_ingest_race.md new file mode 100644 index 0000000000..3a9c2eb5b7 --- /dev/null +++ b/unreleased_history/bug_fixes/manifest_recovery_ingest_race.md @@ -0,0 +1 @@ +Fixed a race between error recovery due to manifest sync or write failure and external SST file ingestion. Both attempt to write a new manifest file, which causes an assertion failure.