Skip to content

Commit

Permalink
Fix data race in GetObsoleteFiles()
Browse files Browse the repository at this point in the history
Summary:
GetObsoleteFiles() and LogAndApply() functions modify obsolete_manifests_ vector
we need to make sure that the mutex is held when we modify the obsolete_manifests_

Test Plan: run the test under TSAN

Reviewers: andrewkr

Reviewed By: andrewkr

Subscribers: andrewkr, dhruba

Differential Revision: https://reviews.facebook.net/D58011
  • Loading branch information
IslamAbdelRahman committed May 11, 2016
1 parent 5c1c904 commit 560358d
Showing 1 changed file with 7 additions and 4 deletions.
11 changes: 7 additions & 4 deletions db/version_set.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2322,10 +2322,6 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
if (s.ok() && new_descriptor_log) {
s = SetCurrentFile(env_, dbname_, pending_manifest_file_number_,
db_options_->disableDataSync ? nullptr : db_directory);
// Leave the old file behind since PurgeObsoleteFiles will take care of it
// later. It's unsafe to delete now since file deletion may be disabled.
obsolete_manifests_.emplace_back(
DescriptorFileName("", manifest_file_number_));
}

if (s.ok()) {
Expand All @@ -2344,6 +2340,13 @@ Status VersionSet::LogAndApply(ColumnFamilyData* column_family_data,
mu->Lock();
}

// Append the old mainfest file to the obsolete_manifests_ list to be deleted
// by PurgeObsoleteFiles later.
if (s.ok() && new_descriptor_log) {
obsolete_manifests_.emplace_back(
DescriptorFileName("", manifest_file_number_));
}

// Install the new version
if (s.ok()) {
if (edit->is_column_family_add_) {
Expand Down

0 comments on commit 560358d

Please sign in to comment.