andreacavalli/rocksdb
0
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

EnableFileDeletions only read field while holding mutex (#7435)

Browse Source 
Summary:
Possible fix for a TSAN issue reported in EnableFileDeletions.
disable_delete_obsolete_files_ should only be accessed holding the db
mutex, but for logging it was being accessed outside holding the mutex,
now fixed.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/7435

Test Plan: existing tests, watch for recurrence

Reviewed By: ltamasi

Differential Revision: D23917578

Pulled By: pdillinger

fbshipit-source-id: 8573025bca3f6fe169b24b87bbfc4ce9667b0482
This commit is contained in:
Peter Dillinger 2020-09-25 13:33:05 -07:00 committed by Facebook GitHub Bot
parent 1a24f4d1d6
commit c8a12aa94b
1 changed files with 5 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
db/db_impl/db_impl_files.cc
View File

@ -58,7 +58,7 @@ Status DBImpl::EnableFileDeletions(bool force) {
// Job id == 0 means that this is not our background process, but rather // Job id == 0 means that this is not our background process, but rather
// user thread // user thread
JobContext job_context(0); JobContext job_context(0);
bool file_deletion_enabled = false; int saved_counter; // initialize on all paths
{ {
InstrumentedMutexLock l(&mutex_); InstrumentedMutexLock l(&mutex_);
if (force) { if (force) {
@ -67,13 +67,13 @@ Status DBImpl::EnableFileDeletions(bool force) {
} else if (disable_delete_obsolete_files_ > 0) { } else if (disable_delete_obsolete_files_ > 0) {
--disable_delete_obsolete_files_; --disable_delete_obsolete_files_;
} }
if (disable_delete_obsolete_files_ == 0) { saved_counter = disable_delete_obsolete_files_;
file_deletion_enabled = true; if (saved_counter == 0) {
FindObsoleteFiles(&job_context, true); FindObsoleteFiles(&job_context, true);
bg_cv_.SignalAll(); bg_cv_.SignalAll();
} }
} }
if (file_deletion_enabled) { if (saved_counter == 0) {
ROCKS_LOG_INFO(immutable_db_options_.info_log, "File Deletions Enabled"); ROCKS_LOG_INFO(immutable_db_options_.info_log, "File Deletions Enabled");
if (job_context.HaveSomethingToDelete()) { if (job_context.HaveSomethingToDelete()) {
PurgeObsoleteFiles(job_context); PurgeObsoleteFiles(job_context);
@ -81,7 +81,7 @@ Status DBImpl::EnableFileDeletions(bool force) {
} else { } else {
ROCKS_LOG_WARN(immutable_db_options_.info_log, ROCKS_LOG_WARN(immutable_db_options_.info_log,
"File Deletions Enable, but not really enabled. Counter: %d", "File Deletions Enable, but not really enabled. Counter: %d",
disable_delete_obsolete_files_); saved_counter);
} }
job_context.Clean(); job_context.Clean();
LogFlush(immutable_db_options_.info_log); LogFlush(immutable_db_options_.info_log);