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
main
Andrew Kryczka 7 years ago committed by Facebook Github Bot
parent ba295cda29
commit 43549c7d59
  1. 9
      db/db_impl_compaction_flush.cc
  2. 7
      db/job_context.h

@ -441,7 +441,8 @@ Status DBImpl::CompactFiles(
} // release the mutex } // release the mutex
// delete unnecessary files if any, this is done outside 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_-- // Have to flush the info logs before bg_compaction_scheduled_--
// because if bg_flush_scheduled_ becomes 0 and the lock is // because if bg_flush_scheduled_ becomes 0 and the lock is
// released, the deconstructor of DB can kick in and destroy all the // 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() // created. Thus, we force full scan in FindObsoleteFiles()
FindObsoleteFiles(&job_context, !s.ok() && !s.IsShutdownInProgress()); FindObsoleteFiles(&job_context, !s.ok() && !s.IsShutdownInProgress());
// delete unnecessary files if any, this is done outside 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()) {
mutex_.Unlock(); mutex_.Unlock();
// Have to flush the info logs before bg_flush_scheduled_-- // Have to flush the info logs before bg_flush_scheduled_--
// because if bg_flush_scheduled_ becomes 0 and the lock is // 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()); FindObsoleteFiles(&job_context, !s.ok() && !s.IsShutdownInProgress());
// delete unnecessary files if any, this is done outside 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()) {
mutex_.Unlock(); mutex_.Unlock();
// Have to flush the info logs before bg_compaction_scheduled_-- // Have to flush the info logs before bg_compaction_scheduled_--
// because if bg_flush_scheduled_ becomes 0 and the lock is // because if bg_flush_scheduled_ becomes 0 and the lock is

@ -81,8 +81,11 @@ struct SuperVersionContext {
struct JobContext { struct JobContext {
inline bool HaveSomethingToDelete() const { inline bool HaveSomethingToDelete() const {
return full_scan_candidate_files.size() || sst_delete_files.size() || return full_scan_candidate_files.size() || sst_delete_files.size() ||
log_delete_files.size() || manifest_delete_files.size() || log_delete_files.size() || manifest_delete_files.size();
memtables_to_free.size() > 0 || logs_to_free.size() > 0 || }
inline bool HaveSomethingToClean() const {
return memtables_to_free.size() > 0 || logs_to_free.size() > 0 ||
superversion_context.HaveSomethingToDelete(); superversion_context.HaveSomethingToDelete();
} }

Loading…
Cancel
Save