From c29050899d4977da2e66e67013a3876363281799 Mon Sep 17 00:00:00 2001 From: Patrick LeBlanc Date: Fri, 22 May 2020 08:06:04 -0400 Subject: [PATCH] Took out the cache repopulate idea. Now we will only have warnings of problems. I realized we can't reliably tell how big the cache is while the system is running. There's a window where write/append has added / is adding a journal file but hasn't told Cache about it yet. This capability will have to wait for now. This shouldn't be a problem because in theory, we will no longer have data whose size is not consistent with metadata stored outside of the file. If we do, it means there was either a hard failure, or SM was killed. Either way, SM will be restarted and the cache will populate its meta fresh then. --- storage-manager/src/Cache.cpp | 13 ------------ storage-manager/src/Cache.h | 5 ----- storage-manager/src/PrefixCache.cpp | 30 +++++++++------------------- storage-manager/src/PrefixCache.h | 9 +-------- storage-manager/src/Synchronizer.cpp | 4 +--- 5 files changed, 11 insertions(+), 50 deletions(-) diff --git a/storage-manager/src/Cache.cpp b/storage-manager/src/Cache.cpp index de7b22c16..3f9db3460 100644 --- a/storage-manager/src/Cache.cpp +++ b/storage-manager/src/Cache.cpp @@ -356,19 +356,6 @@ void Cache::configListener() } } -void Cache::repopulate() -{ - boost::unique_lock sl(lru_mutex); - - for (auto &pcache : prefixCaches) - pcache.second->repopulate(); -} - -void Cache::repopulate(const boost::filesystem::path &p) -{ - getPCache(p).repopulate(); -} - } diff --git a/storage-manager/src/Cache.h b/storage-manager/src/Cache.h index 3ae57a342..12396550a 100644 --- a/storage-manager/src/Cache.h +++ b/storage-manager/src/Cache.h @@ -92,11 +92,6 @@ class Cache : public boost::noncopyable , public ConfigListener void shutdown(); void printKPIs() const; - // Used to update accounting variables in the PrefixCaches when a potential error - // is detected. - void repopulate(); - void repopulate(const boost::filesystem::path &prefix); - // test helpers const boost::filesystem::path &getCachePath() const; const boost::filesystem::path &getJournalPath() const; diff --git a/storage-manager/src/PrefixCache.cpp b/storage-manager/src/PrefixCache.cpp index 99a65dbba..85e5c68c7 100644 --- a/storage-manager/src/PrefixCache.cpp +++ b/storage-manager/src/PrefixCache.cpp @@ -127,13 +127,7 @@ PrefixCache::~PrefixCache() */ } -void PrefixCache::repopulate() -{ - lru_mutex.lock(); - populate(false); -} - -void PrefixCache::populate(bool useSync) +void PrefixCache::populate() { Synchronizer *sync = Synchronizer::get(); bf::directory_iterator dir(cachePrefix); @@ -151,15 +145,13 @@ void PrefixCache::populate(bool useSync) auto last = lru.end(); m_lru.insert(--last); currentCacheSize += bf::file_size(*dir); - if (useSync) - newObjects.push_back(p.filename().string()); + newObjects.push_back(p.filename().string()); } else if (p != cachePrefix/downloader->getTmpPath()) logger->log(LOG_WARNING, "Cache: found something in the cache that does not belong '%s'", p.string().c_str()); ++dir; } - if (useSync) - sync->newObjects(firstDir, newObjects); + sync->newObjects(firstDir, newObjects); newObjects.clear(); // account for what's in the journal dir @@ -174,8 +166,7 @@ void PrefixCache::populate(bool useSync) { size_t s = bf::file_size(*dir); currentCacheSize += s; - if (useSync) - newJournals.push_back(pair(p.stem().string(), s)); + newJournals.push_back(pair(p.stem().string(), s)); } else logger->log(LOG_WARNING, "Cache: found a file in the journal dir that does not belong '%s'", p.string().c_str()); @@ -185,8 +176,7 @@ void PrefixCache::populate(bool useSync) ++dir; } lru_mutex.unlock(); - if (useSync) - sync->newJournalEntries(firstDir, newJournals); + sync->newJournalEntries(firstDir, newJournals); } // be careful using this! SM should be idle. No ongoing reads or writes. @@ -399,10 +389,9 @@ void PrefixCache::deletedJournal(size_t size) else { ostringstream oss; - oss << "PrefixCache::deletedJournal(): Detected an accounting error." << - " Reloading cache metadata, this will pause IO activity briefly."; + oss << "PrefixCache::deletedJournal(): Detected an accounting error."; logger->log(LOG_WARNING, oss.str().c_str()); - populate(false); + currentCacheSize = 0; } } @@ -425,10 +414,9 @@ void PrefixCache::deletedObject(const string &key, size_t size) else { ostringstream oss; - oss << "PrefixCache::deletedObject(): Detected an accounting error." << - " Reloading cache metadata, this will pause IO activity briefly."; + oss << "PrefixCache::deletedObject(): Detected an accounting error."; logger->log(LOG_WARNING, oss.str().c_str()); - populate(false); + currentCacheSize = 0; } } } diff --git a/storage-manager/src/PrefixCache.h b/storage-manager/src/PrefixCache.h index da6b7fb66..1121275c7 100644 --- a/storage-manager/src/PrefixCache.h +++ b/storage-manager/src/PrefixCache.h @@ -77,11 +77,6 @@ class PrefixCache : public boost::noncopyable size_t getMaxCacheSize() const; void shutdown(); - // clears out cache structures and reloads them from cache/journal dir contents - // needed to potentially repair the cache's accounting error after detecting - // an error. - void repopulate(); - // test helpers const boost::filesystem::path &getCachePath(); const boost::filesystem::path &getJournalPath(); @@ -102,9 +97,7 @@ class PrefixCache : public boost::noncopyable SMLogging *logger; Downloader *downloader; - // useSync makes populate() tell Synchronizer about what it finds. - // set it to false when the system is already fully up. - void populate(bool useSync = true); + void populate(); void _makeSpace(size_t size); /* The main PrefixCache structures */ diff --git a/storage-manager/src/Synchronizer.cpp b/storage-manager/src/Synchronizer.cpp index b8e07ae39..20bd39213 100644 --- a/storage-manager/src/Synchronizer.cpp +++ b/storage-manager/src/Synchronizer.cpp @@ -706,10 +706,8 @@ void Synchronizer::synchronizeWithJournal(const string &sourceFile, list ostringstream oss; oss << "Synchronizer::synchronizeWithJournal(): detected a mismatch between file size and " << "length stored in the object name. object name = " << cloudKey << " length-in-name = " << - MetadataFile::getLengthFromKey(cloudKey) << " real-length = " << bf::file_size(oldCachePath) - << ". Reloading cache metadata, this will pause IO activity briefly."; + MetadataFile::getLengthFromKey(cloudKey) << " real-length = " << bf::file_size(oldCachePath); logger->log(LOG_WARNING, oss.str().c_str()); - cache->repopulate(prefix); } }