From 43549c7d59c40dee7af1c955cb23a7dfa1227e5a Mon Sep 17 00:00:00 2001 From: Andrew Kryczka Date: Fri, 12 Jan 2018 13:16:39 -0800 Subject: [PATCH] Prevent unnecessary calls to PurgeObsoleteFiles Summary: Split `JobContext::HaveSomethingToDelete` into two functions: itself and `JobContext::HaveSomethingToClean`. Now we won't call `DBImpl::PurgeObsoleteFiles` in cases where we really just need to call `JobContext::Clean`. The change is needed because I want to track pending calls to `PurgeObsoleteFiles` for a bug fix, which is much simpler if we only call it after `FindObsoleteFiles` finds files to delete. Closes https://github.com/facebook/rocksdb/pull/3350 Differential Revision: D6690609 Pulled By: ajkr fbshipit-source-id: 61502e7469288afe16a663a1b7df345baeaf246f --- db/db_impl_compaction_flush.cc | 9 ++++++--- db/job_context.h | 7 +++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/db/db_impl_compaction_flush.cc b/db/db_impl_compaction_flush.cc index 02f35df21..c4b05a594 100644 --- a/db/db_impl_compaction_flush.cc +++ b/db/db_impl_compaction_flush.cc @@ -441,7 +441,8 @@ Status DBImpl::CompactFiles( } // release the mutex // delete unnecessary files if any, this is done outside the mutex - if (job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { + if (job_context.HaveSomethingToClean() || + job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { // Have to flush the info logs before bg_compaction_scheduled_-- // because if bg_flush_scheduled_ becomes 0 and the lock is // released, the deconstructor of DB can kick in and destroy all the @@ -1303,7 +1304,8 @@ void DBImpl::BackgroundCallFlush() { // created. Thus, we force full scan in FindObsoleteFiles() FindObsoleteFiles(&job_context, !s.ok() && !s.IsShutdownInProgress()); // delete unnecessary files if any, this is done outside the mutex - if (job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { + if (job_context.HaveSomethingToClean() || + job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { mutex_.Unlock(); // Have to flush the info logs before bg_flush_scheduled_-- // because if bg_flush_scheduled_ becomes 0 and the lock is @@ -1384,7 +1386,8 @@ void DBImpl::BackgroundCallCompaction(PrepickedCompaction* prepicked_compaction, FindObsoleteFiles(&job_context, !s.ok() && !s.IsShutdownInProgress()); // delete unnecessary files if any, this is done outside the mutex - if (job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { + if (job_context.HaveSomethingToClean() || + job_context.HaveSomethingToDelete() || !log_buffer.IsEmpty()) { mutex_.Unlock(); // Have to flush the info logs before bg_compaction_scheduled_-- // because if bg_flush_scheduled_ becomes 0 and the lock is diff --git a/db/job_context.h b/db/job_context.h index e0af71031..0700cfcee 100644 --- a/db/job_context.h +++ b/db/job_context.h @@ -81,8 +81,11 @@ struct SuperVersionContext { struct JobContext { inline bool HaveSomethingToDelete() const { return full_scan_candidate_files.size() || sst_delete_files.size() || - log_delete_files.size() || manifest_delete_files.size() || - memtables_to_free.size() > 0 || logs_to_free.size() > 0 || + log_delete_files.size() || manifest_delete_files.size(); + } + + inline bool HaveSomethingToClean() const { + return memtables_to_free.size() > 0 || logs_to_free.size() > 0 || superversion_context.HaveSomethingToDelete(); }