From 7ef7b147f98c5ba14111143dc7852e2dfd531b7c Mon Sep 17 00:00:00 2001 From: Victoria McGrath Date: Tue, 23 Nov 2021 16:00:10 -0500 Subject: [PATCH 1/3] Disabled NvmCacheState when NVM cache is not enabled. --- cachelib/allocator/CacheAllocator-inl.h | 17 +++++++++++------ cachelib/allocator/CacheAllocatorConfig.h | 7 +++++++ cachelib/allocator/NvmCacheState.cpp | 8 +++++--- cachelib/allocator/NvmCacheState.h | 6 +++++- 4 files changed, 28 insertions(+), 10 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index fc485c2ae9..8d2075df66 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -51,7 +51,8 @@ CacheAllocator::CacheAllocator(Config config) std::make_shared()), cacheCreationTime_{util::getCurrentTimeSec()}, nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled()} { + config_.isNvmCacheTruncateAllocSizeEnabled(), + config_.isNvmCacheEnabled()} { // TODO(MEMORY_TIER) if (std::holds_alternative( memoryTierConfigs[0].getShmTypeOpts())) { @@ -99,7 +100,8 @@ CacheAllocator::CacheAllocator(SharedMemNewT, Config config) std::make_shared()), cacheCreationTime_{util::getCurrentTimeSec()}, nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled()} { + config_.isNvmCacheTruncateAllocSizeEnabled(), + config_.isNvmCacheEnabled()} { initCommon(false); shmManager_->removeShm(detail::kShmInfoName, PosixSysVSegmentOpts(config_.isUsingPosixShm())); @@ -136,7 +138,8 @@ CacheAllocator::CacheAllocator(SharedMemAttachT, Config config) std::make_shared()), cacheCreationTime_{*metadata_.cacheCreationTime_ref()}, nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled()} { + config_.isNvmCacheTruncateAllocSizeEnabled(), + config_.isNvmCacheEnabled()} { for (auto pid : *metadata_.compactCachePools_ref()) { isCompactCachePool_[pid] = true; } @@ -207,7 +210,7 @@ CacheAllocator::restoreCCacheManager() { template void CacheAllocator::initCommon(bool dramCacheAttached) { - if (config_.nvmConfig.has_value()) { + if (config_.isNvmCacheEnabled()) { if (config_.nvmCacheAP) { nvmAdmissionPolicy_ = config_.nvmCacheAP; } else if (config_.rejectFirstAPNumEntries) { @@ -230,7 +233,7 @@ void CacheAllocator::initCommon(bool dramCacheAttached) { template void CacheAllocator::initNvmCache(bool dramCacheAttached) { - if (!config_.nvmConfig.has_value()) { + if (!config_.isNvmCacheEnabled()) { return; } @@ -3252,8 +3255,10 @@ GlobalCacheStats CacheAllocator::getGlobalCacheStats() const { const uint64_t currTime = util::getCurrentTimeSec(); ret.ramUpTime = currTime - cacheCreationTime_; - ret.nvmUpTime = currTime - nvmCacheState_.getCreationTime(); ret.nvmCacheEnabled = nvmCache_ ? nvmCache_->isEnabled() : false; + if (ret.nvmCacheEnabled) { + ret.nvmUpTime = currTime - nvmCacheState_.getCreationTime(); + } ret.reaperStats = getReaperStats(); ret.numActiveHandles = getNumActiveHandles(); diff --git a/cachelib/allocator/CacheAllocatorConfig.h b/cachelib/allocator/CacheAllocatorConfig.h index cb578717cb..a5d2058687 100644 --- a/cachelib/allocator/CacheAllocatorConfig.h +++ b/cachelib/allocator/CacheAllocatorConfig.h @@ -89,6 +89,8 @@ class CacheAllocatorConfig { // Config for NvmCache. If enabled, cachelib will also make use of flash. CacheAllocatorConfig& enableNvmCache(NvmCacheConfig config); + bool isNvmCacheEnabled() const; + // enable the reject first admission policy through its parameters // @param numEntries the number of entries to track across all splits // @param numSplits the number of splits. we drop a whole split by @@ -660,6 +662,11 @@ CacheAllocatorConfig& CacheAllocatorConfig::enableNvmCache( return *this; } +template +bool CacheAllocatorConfig::isNvmCacheEnabled() const { + return nvmConfig.has_value(); +} + template CacheAllocatorConfig& CacheAllocatorConfig::setNvmCacheAdmissionPolicy( std::shared_ptr> policy) { diff --git a/cachelib/allocator/NvmCacheState.cpp b/cachelib/allocator/NvmCacheState.cpp index f8bfb55103..d189f98de4 100644 --- a/cachelib/allocator/NvmCacheState.cpp +++ b/cachelib/allocator/NvmCacheState.cpp @@ -75,12 +75,14 @@ std::string NvmCacheState::getNvmCacheStateFilePath( NvmCacheState::NvmCacheState(const std::string& cacheDir, bool encryptionEnabled, - bool truncateAllocSize) + bool truncateAllocSize, + bool nvmCacheEnabled) : cacheDir_(cacheDir), creationTime_{util::getCurrentTimeSec()}, encryptionEnabled_{encryptionEnabled}, - truncateAllocSize_{truncateAllocSize} { - if (cacheDir_.empty()) { + truncateAllocSize_{truncateAllocSize}, + nvmCacheEnabled_(nvmCacheEnabled) { + if (cacheDir_.empty() || !nvmCacheEnabled_) { return; } diff --git a/cachelib/allocator/NvmCacheState.h b/cachelib/allocator/NvmCacheState.h index c11942c123..49f96600ab 100644 --- a/cachelib/allocator/NvmCacheState.h +++ b/cachelib/allocator/NvmCacheState.h @@ -46,7 +46,8 @@ class NvmCacheState { // Intialize the object for the cacheDir. explicit NvmCacheState(const std::string& cacheDir, bool encryptionEnabled, - bool truncateAllocSize); + bool truncateAllocSize, + bool nvmCacheEnabled = true); // returns the time when the nvmcache associated with the cacheDir was // created @@ -100,6 +101,9 @@ class NvmCacheState { // if enabled, only store user-requested size into nvm cache bool truncateAllocSize_{false}; + // is NVM cache enabled + bool nvmCacheEnabled_{false}; + // a stream for writing metadata is created and kept open throughout the // lifetime of this object. This resolves any issue with a process's // permission to write metadata if the user downgrades the process's perm. From 36fef0e843d3a2c2d3c9427705b0880f387fd07d Mon Sep 17 00:00:00 2001 From: Victoria McGrath Date: Wed, 24 Nov 2021 13:46:51 -0500 Subject: [PATCH 2/3] Changed NvmCacheStas_ to std::optional --- cachelib/allocator/CacheAllocator-inl.h | 30 ++++++++++--------------- cachelib/allocator/CacheAllocator.h | 4 ++-- cachelib/allocator/NvmCacheState.cpp | 8 +++---- cachelib/allocator/NvmCacheState.h | 6 +---- 4 files changed, 18 insertions(+), 30 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 8d2075df66..9c9b2b8efe 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -49,10 +49,7 @@ CacheAllocator::CacheAllocator(Config config) [this](Item* it) -> ItemHandle { return acquire(it); })), chainedItemLocks_(config_.chainedItemsLockPower, std::make_shared()), - cacheCreationTime_{util::getCurrentTimeSec()}, - nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled(), - config_.isNvmCacheEnabled()} { + cacheCreationTime_{util::getCurrentTimeSec()} { // TODO(MEMORY_TIER) if (std::holds_alternative( memoryTierConfigs[0].getShmTypeOpts())) { @@ -98,10 +95,7 @@ CacheAllocator::CacheAllocator(SharedMemNewT, Config config) [this](Item* it) -> ItemHandle { return acquire(it); })), chainedItemLocks_(config_.chainedItemsLockPower, std::make_shared()), - cacheCreationTime_{util::getCurrentTimeSec()}, - nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled(), - config_.isNvmCacheEnabled()} { + cacheCreationTime_{util::getCurrentTimeSec()} { initCommon(false); shmManager_->removeShm(detail::kShmInfoName, PosixSysVSegmentOpts(config_.isUsingPosixShm())); @@ -136,10 +130,7 @@ CacheAllocator::CacheAllocator(SharedMemAttachT, Config config) [this](Item* it) -> ItemHandle { return acquire(it); })), chainedItemLocks_(config_.chainedItemsLockPower, std::make_shared()), - cacheCreationTime_{*metadata_.cacheCreationTime_ref()}, - nvmCacheState_{config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), - config_.isNvmCacheTruncateAllocSizeEnabled(), - config_.isNvmCacheEnabled()} { + cacheCreationTime_{*metadata_.cacheCreationTime_ref()} { for (auto pid : *metadata_.compactCachePools_ref()) { isCompactCachePool_[pid] = true; } @@ -237,20 +228,23 @@ void CacheAllocator::initNvmCache(bool dramCacheAttached) { return; } + nvmCacheState_.emplace(NvmCacheState(config_.cacheDir, config_.isNvmCacheEncryptionEnabled(), + config_.isNvmCacheTruncateAllocSizeEnabled())); + // for some usecases that create pools, restoring nvmcache when dram cache // is not persisted is not supported. const bool shouldDrop = config_.dropNvmCacheOnShmNew && !dramCacheAttached; // if we are dealing with persistency, cache directory should be enabled const bool truncate = config_.cacheDir.empty() || - nvmCacheState_.shouldStartFresh() || shouldDrop; + nvmCacheState_.value().shouldStartFresh() || shouldDrop; if (truncate) { - nvmCacheState_.markTruncated(); + nvmCacheState_.value().markTruncated(); } nvmCache_ = std::make_unique(*this, *config_.nvmConfig, truncate); if (!config_.cacheDir.empty()) { - nvmCacheState_.clearPrevState(); + nvmCacheState_.value().clearPrevState(); } } @@ -3060,7 +3054,7 @@ std::optional CacheAllocator::saveNvmCache() { return false; } - nvmCacheState_.markSafeShutDown(); + nvmCacheState_.value().markSafeShutDown(); return true; } @@ -3256,8 +3250,8 @@ GlobalCacheStats CacheAllocator::getGlobalCacheStats() const { const uint64_t currTime = util::getCurrentTimeSec(); ret.ramUpTime = currTime - cacheCreationTime_; ret.nvmCacheEnabled = nvmCache_ ? nvmCache_->isEnabled() : false; - if (ret.nvmCacheEnabled) { - ret.nvmUpTime = currTime - nvmCacheState_.getCreationTime(); + if (nvmCacheState_.has_value()) { + ret.nvmUpTime = currTime - nvmCacheState_.value().getCreationTime(); } ret.reaperStats = getReaperStats(); ret.numActiveHandles = getNumActiveHandles(); diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index 27cf7b0ca6..763e3de166 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -969,7 +969,7 @@ class CacheAllocator : public CacheBase { // @return time when the cache was created. time_t getCacheCreationTime() const noexcept { return cacheCreationTime_; } time_t getNVMCacheCreationTime() const { - return nvmCacheState_.getCreationTime(); + return nvmCacheState_.value().getCreationTime(); } // Inspects the cache without changing its state. @@ -1812,7 +1812,7 @@ class CacheAllocator : public CacheBase { folly::ThreadLocal ring_; // state for the nvmcache - NvmCacheState nvmCacheState_; + std::optional nvmCacheState_{}; // admission policy for nvmcache std::shared_ptr> nvmAdmissionPolicy_; diff --git a/cachelib/allocator/NvmCacheState.cpp b/cachelib/allocator/NvmCacheState.cpp index d189f98de4..f8bfb55103 100644 --- a/cachelib/allocator/NvmCacheState.cpp +++ b/cachelib/allocator/NvmCacheState.cpp @@ -75,14 +75,12 @@ std::string NvmCacheState::getNvmCacheStateFilePath( NvmCacheState::NvmCacheState(const std::string& cacheDir, bool encryptionEnabled, - bool truncateAllocSize, - bool nvmCacheEnabled) + bool truncateAllocSize) : cacheDir_(cacheDir), creationTime_{util::getCurrentTimeSec()}, encryptionEnabled_{encryptionEnabled}, - truncateAllocSize_{truncateAllocSize}, - nvmCacheEnabled_(nvmCacheEnabled) { - if (cacheDir_.empty() || !nvmCacheEnabled_) { + truncateAllocSize_{truncateAllocSize} { + if (cacheDir_.empty()) { return; } diff --git a/cachelib/allocator/NvmCacheState.h b/cachelib/allocator/NvmCacheState.h index 49f96600ab..c11942c123 100644 --- a/cachelib/allocator/NvmCacheState.h +++ b/cachelib/allocator/NvmCacheState.h @@ -46,8 +46,7 @@ class NvmCacheState { // Intialize the object for the cacheDir. explicit NvmCacheState(const std::string& cacheDir, bool encryptionEnabled, - bool truncateAllocSize, - bool nvmCacheEnabled = true); + bool truncateAllocSize); // returns the time when the nvmcache associated with the cacheDir was // created @@ -101,9 +100,6 @@ class NvmCacheState { // if enabled, only store user-requested size into nvm cache bool truncateAllocSize_{false}; - // is NVM cache enabled - bool nvmCacheEnabled_{false}; - // a stream for writing metadata is created and kept open throughout the // lifetime of this object. This resolves any issue with a process's // permission to write metadata if the user downgrades the process's perm. From 3d70a3ec36c14d3683e00034c5fc05f5a02d4946 Mon Sep 17 00:00:00 2001 From: Victoria McGrath Date: Mon, 29 Nov 2021 11:06:50 -0500 Subject: [PATCH 3/3] Changed getNVMCacheCreationTime() to return cacheCreationTime if NVM cache is not set up --- cachelib/allocator/CacheAllocator-inl.h | 4 +--- cachelib/allocator/CacheAllocator.h | 11 ++++++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/cachelib/allocator/CacheAllocator-inl.h b/cachelib/allocator/CacheAllocator-inl.h index 9c9b2b8efe..f178a0999a 100644 --- a/cachelib/allocator/CacheAllocator-inl.h +++ b/cachelib/allocator/CacheAllocator-inl.h @@ -3250,9 +3250,7 @@ GlobalCacheStats CacheAllocator::getGlobalCacheStats() const { const uint64_t currTime = util::getCurrentTimeSec(); ret.ramUpTime = currTime - cacheCreationTime_; ret.nvmCacheEnabled = nvmCache_ ? nvmCache_->isEnabled() : false; - if (nvmCacheState_.has_value()) { - ret.nvmUpTime = currTime - nvmCacheState_.value().getCreationTime(); - } + ret.nvmUpTime = currTime - getNVMCacheCreationTime(); ret.reaperStats = getReaperStats(); ret.numActiveHandles = getNumActiveHandles(); diff --git a/cachelib/allocator/CacheAllocator.h b/cachelib/allocator/CacheAllocator.h index 763e3de166..abdc13485e 100644 --- a/cachelib/allocator/CacheAllocator.h +++ b/cachelib/allocator/CacheAllocator.h @@ -968,8 +968,17 @@ class CacheAllocator : public CacheBase { // // @return time when the cache was created. time_t getCacheCreationTime() const noexcept { return cacheCreationTime_; } + + // unix timestamp when the NVM cache was created. If NVM cahce isn't enaled, + // the cache creation time is returned instead. + // + // @return time when the NVM cache was created. time_t getNVMCacheCreationTime() const { - return nvmCacheState_.value().getCreationTime(); + auto result = getCacheCreationTime(); + if (nvmCacheState_.has_value()) { + result = nvmCacheState_.value().getCreationTime(); + } + return result; } // Inspects the cache without changing its state.