diff --git a/db/column_family.cc b/db/column_family.cc index e749cb9ee..a2885515c 100644 --- a/db/column_family.cc +++ b/db/column_family.cc @@ -75,11 +75,6 @@ ColumnFamilyHandleImpl::~ColumnFamilyHandleImpl() { bool defer_purge = db_->immutable_db_options().avoid_unnecessary_blocking_io; db_->PurgeObsoleteFiles(job_context, defer_purge); - if (defer_purge) { - mutex_->Lock(); - db_->SchedulePurge(); - mutex_->Unlock(); - } } job_context.Clean(); } diff --git a/db/db_filesnapshot.cc b/db/db_filesnapshot.cc index c9694c751..35d75c823 100644 --- a/db/db_filesnapshot.cc +++ b/db/db_filesnapshot.cc @@ -127,31 +127,30 @@ Status DBImpl::GetLiveFiles(std::vector& ret, } Status DBImpl::GetSortedWalFiles(VectorLogPtr& files) { + // If caller disabled deletions, this function should return files that are + // guaranteed not to be deleted until deletions are re-enabled. We need to + // wait for pending purges to finish since WalManager doesn't know which + // files are going to be purged. Additional purges won't be scheduled as + // long as deletions are disabled (so the below loop must terminate). + // Also note that we disable deletions anyway to avoid the case where a + // file is deleted in the middle of the scan, causing IO error. + Status deletions_disabled = DisableFileDeletions(); { - // If caller disabled deletions, this function should return files that are - // guaranteed not to be deleted until deletions are re-enabled. We need to - // wait for pending purges to finish since WalManager doesn't know which - // files are going to be purged. Additional purges won't be scheduled as - // long as deletions are disabled (so the below loop must terminate). InstrumentedMutexLock l(&mutex_); - while (disable_delete_obsolete_files_ > 0 && - (pending_purge_obsolete_files_ > 0 || bg_purge_scheduled_ > 0)) { + while (pending_purge_obsolete_files_ > 0 || bg_purge_scheduled_ > 0) { bg_cv_.Wait(); } } - // Disable deletion in order to avoid the case where a file is deleted in - // the middle of the process so IO error is returned. - Status s = DisableFileDeletions(); - bool file_deletion_supported = !s.IsNotSupported(); - if (s.ok() || !file_deletion_supported) { - s = wal_manager_.GetSortedWalFiles(files); - if (file_deletion_supported) { - Status s2 = EnableFileDeletions(false); - if (!s2.ok() && s.ok()) { - s = s2; - } - } + Status s = wal_manager_.GetSortedWalFiles(files); + + // DisableFileDeletions / EnableFileDeletions not supported in read-only DB + if (deletions_disabled.ok()) { + Status s2 = EnableFileDeletions(/*force*/ false); + assert(s2.ok()); + s2.PermitUncheckedError(); + } else { + assert(deletions_disabled.IsNotSupported()); } return s; diff --git a/db/db_impl/db_impl.cc b/db/db_impl/db_impl.cc index 190ad54b2..1ff9880b8 100644 --- a/db/db_impl/db_impl.cc +++ b/db/db_impl/db_impl.cc @@ -1546,6 +1546,8 @@ void DBImpl::BackgroundCallPurge() { mutex_.Lock(); } + assert(bg_purge_scheduled_ > 0); + // Can't use iterator to go over purge_files_ because inside the loop we're // unlocking the mutex that protects purge_files_. while (!purge_files_.empty()) { @@ -1613,17 +1615,7 @@ static void CleanupIteratorState(void* arg1, void* /*arg2*/) { delete state->super_version; } if (job_context.HaveSomethingToDelete()) { - if (state->background_purge) { - // PurgeObsoleteFiles here does not delete files. Instead, it adds the - // files to be deleted to a job queue, and deletes it in a separate - // background thread. - state->db->PurgeObsoleteFiles(job_context, true /* schedule only */); - state->mu->Lock(); - state->db->SchedulePurge(); - state->mu->Unlock(); - } else { - state->db->PurgeObsoleteFiles(job_context); - } + state->db->PurgeObsoleteFiles(job_context, state->background_purge); } job_context.Clean(); } diff --git a/db/db_impl/db_impl_files.cc b/db/db_impl/db_impl_files.cc index 6007d2ffb..7ce1a0ae4 100644 --- a/db/db_impl/db_impl_files.cc +++ b/db/db_impl/db_impl_files.cc @@ -56,6 +56,8 @@ Status DBImpl::DisableFileDeletions() { return s; } +// FIXME: can be inconsistent with DisableFileDeletions in cases like +// DBImplReadOnly Status DBImpl::DisableFileDeletionsWithLock() { mutex_.AssertHeld(); ++disable_delete_obsolete_files_; @@ -642,6 +644,11 @@ void DBImpl::PurgeObsoleteFiles(JobContext& state, bool schedule_only) { InstrumentedMutexLock l(&mutex_); --pending_purge_obsolete_files_; assert(pending_purge_obsolete_files_ >= 0); + if (schedule_only) { + // Must change from pending_purge_obsolete_files_ to bg_purge_scheduled_ + // while holding mutex (for GetSortedWalFiles() etc.) + SchedulePurge(); + } if (pending_purge_obsolete_files_ == 0) { bg_cv_.SignalAll(); } @@ -657,11 +664,6 @@ void DBImpl::DeleteObsoleteFiles() { if (job_context.HaveSomethingToDelete()) { bool defer_purge = immutable_db_options_.avoid_unnecessary_blocking_io; PurgeObsoleteFiles(job_context, defer_purge); - if (defer_purge) { - mutex_.Lock(); - SchedulePurge(); - mutex_.Unlock(); - } } job_context.Clean(); mutex_.Lock();