Simplify unlocking in DeleteObsoleteFiles.

A recent change (4cb80b7ddc) to DBImpl::DeleteObsoleteFiles
unlocked DBImpl::mutex_ while deleting files to allow for
greater concurrency. This change improves on the prior in
a few areas:

1. The table is evicted from the table cache before unlocking
   the mutex. This should only improve performance.
2. This implementation is slightly simpler, but at the cost of
   a bit more memory usage.
3. A comment adding more detail as to why the mutex is being
   unlocked and why it is safe to do so.

PiperOrigin-RevId: 253111645
This commit is contained in:
Chris Mumford 2019-06-13 14:59:06 -07:00
parent 046216a7ca
commit 53e280b568

View File

@ -229,32 +229,27 @@ void DBImpl::DeleteObsoleteFiles() {
return; return;
} }
const uint64_t log_number = versions_->LogNumber();
const uint64_t prev_log_number = versions_->PrevLogNumber();
const uint64_t manifest_file_number = versions_->ManifestFileNumber();
// Make a set of all of the live files // Make a set of all of the live files
std::set<uint64_t> live = pending_outputs_; std::set<uint64_t> live = pending_outputs_;
versions_->AddLiveFiles(&live); versions_->AddLiveFiles(&live);
std::vector<std::string> filenames; std::vector<std::string> filenames;
env_->GetChildren(dbname_, &filenames); // Ignoring errors on purpose env_->GetChildren(dbname_, &filenames); // Ignoring errors on purpose
// Unlock while deleting obsolete files
mutex_.Unlock();
uint64_t number; uint64_t number;
FileType type; FileType type;
for (size_t i = 0; i < filenames.size(); i++) { std::vector<std::string> files_to_delete;
if (ParseFileName(filenames[i], &number, &type)) { for (std::string& filename : filenames) {
if (ParseFileName(filename, &number, &type)) {
bool keep = true; bool keep = true;
switch (type) { switch (type) {
case kLogFile: case kLogFile:
keep = ((number >= log_number) || (number == prev_log_number)); keep = ((number >= versions_->LogNumber()) ||
(number == versions_->PrevLogNumber()));
break; break;
case kDescriptorFile: case kDescriptorFile:
// Keep my manifest file, and any newer incarnations' // Keep my manifest file, and any newer incarnations'
// (in case there is a race that allows other incarnations) // (in case there is a race that allows other incarnations)
keep = (number >= manifest_file_number); keep = (number >= versions_->ManifestFileNumber());
break; break;
case kTableFile: case kTableFile:
keep = (live.find(number) != live.end()); keep = (live.find(number) != live.end());
@ -272,15 +267,23 @@ void DBImpl::DeleteObsoleteFiles() {
} }
if (!keep) { if (!keep) {
files_to_delete.push_back(std::move(filename));
if (type == kTableFile) { if (type == kTableFile) {
table_cache_->Evict(number); table_cache_->Evict(number);
} }
Log(options_.info_log, "Delete type=%d #%lld\n", static_cast<int>(type), Log(options_.info_log, "Delete type=%d #%lld\n", static_cast<int>(type),
static_cast<unsigned long long>(number)); static_cast<unsigned long long>(number));
env_->DeleteFile(dbname_ + "/" + filenames[i]);
} }
} }
} }
// While deleting all files unblock other threads. All files being deleted
// have unique names which will not collide with newly created files and
// are therefore safe to delete while allowing other threads to proceed.
mutex_.Unlock();
for (const std::string& filename : files_to_delete) {
env_->DeleteFile(dbname_ + "/" + filename);
}
mutex_.Lock(); mutex_.Lock();
} }