Bug fix/simplify cache setup (#9341)

arangod/RocksDBEngine/RocksDBCollection.cpp

@@ -76,7 +76,6 @@ RocksDBCollection::RocksDBCollection(LogicalCollection& collection,
       _revisionId(0),
       _primaryIndex(nullptr),
       _cache(nullptr),
-      _cachePresent(false),
       _cacheEnabled(
           !collection.system() &&
           basics::VelocyPackHelper::readBooleanValue(info, "cacheEnabled", false) &&
@@ -107,7 +106,6 @@ RocksDBCollection::RocksDBCollection(LogicalCollection& collection,
       _revisionId(0),
       _primaryIndex(nullptr),
       _cache(nullptr),
-      _cachePresent(false),
       _cacheEnabled(static_cast<RocksDBCollection const*>(physical)->_cacheEnabled &&
                     CacheManagerFeature::MANAGER != nullptr),
       _numIndexCreations(0) {
@@ -190,7 +188,7 @@ int RocksDBCollection::close() {
 void RocksDBCollection::load() {
   if (_cacheEnabled) {
     createCache();
-    if (_cachePresent) {
+    if (_cache) {
       uint64_t numDocs = numberDocuments();
       if (numDocs > 0) {
         _cache->sizeHint(static_cast<uint64_t>(0.3 * numDocs));
@@ -206,7 +204,7 @@ void RocksDBCollection::load() {
 void RocksDBCollection::unload() {
   if (useCache()) {
     destroyCache();
-    TRI_ASSERT(!_cachePresent);
+    TRI_ASSERT(_cache.get() == nullptr);
   }
   READ_LOCKER(guard, _indexesLock);
   for (auto it : _indexes) {
@@ -1738,7 +1736,7 @@ void RocksDBCollection::estimateSize(velocypack::Builder& builder) {
 }
 
 void RocksDBCollection::createCache() const {
-  if (!_cacheEnabled || _cachePresent || _logicalCollection.isAStub() ||
+  if (!_cacheEnabled || _cache || _logicalCollection.isAStub() ||
       ServerState::instance()->isCoordinator()) {
     // we leave this if we do not need the cache
     // or if cache already created
@@ -1750,12 +1748,11 @@ void RocksDBCollection::createCache() const {
   TRI_ASSERT(CacheManagerFeature::MANAGER != nullptr);
   LOG_TOPIC("f5df2", DEBUG, Logger::CACHE) << "Creating document cache";
   _cache = CacheManagerFeature::MANAGER->createCache(cache::CacheType::Transactional);
-  _cachePresent = (_cache.get() != nullptr);
   TRI_ASSERT(_cacheEnabled);
 }
 
 void RocksDBCollection::destroyCache() const {
-  if (!_cachePresent) {
+  if (!_cache) {
     return;
   }
   TRI_ASSERT(CacheManagerFeature::MANAGER != nullptr);
@@ -1764,7 +1761,6 @@ void RocksDBCollection::destroyCache() const {
   LOG_TOPIC("7137b", DEBUG, Logger::CACHE) << "Destroying document cache";
   CacheManagerFeature::MANAGER->destroyCache(_cache);
   _cache.reset();
-  _cachePresent = false;
 }
 
 // blacklist given key from transactional cache

arangod/RocksDBEngine/RocksDBCollection.h

@@ -228,7 +228,7 @@ class RocksDBCollection final : public PhysicalCollection {
 
   /// is this collection using a cache
   inline bool useCache() const noexcept {
-    return (_cacheEnabled && _cachePresent);
+    return (_cacheEnabled && _cache);
   }
   
   /// @brief track key in file
@@ -254,9 +254,6 @@ class RocksDBCollection final : public PhysicalCollection {
   /// @brief document cache (optional)
   mutable std::shared_ptr<cache::Cache> _cache;
 
-  // we use this boolean for testing whether _cache is set.
-  // it's quicker than accessing the shared_ptr each time
-  mutable bool _cachePresent;
   bool _cacheEnabled;
   /// @brief number of index creations in progress
   std::atomic<int> _numIndexCreations;

arangod/RocksDBEngine/RocksDBIndex.cpp

@@ -67,7 +67,6 @@ RocksDBIndex::RocksDBIndex(TRI_idx_iid_t id, LogicalCollection& collection,
       _objectId(::ensureObjectId(objectId)),
       _cf(cf),
       _cache(nullptr),
-      _cachePresent(false),
       _cacheEnabled(useCache && !collection.system() && CacheManagerFeature::MANAGER != nullptr) {
   TRI_ASSERT(cf != nullptr && cf != RocksDBColumnFamily::definitions());
 
@@ -87,7 +86,6 @@ RocksDBIndex::RocksDBIndex(TRI_idx_iid_t id, LogicalCollection& collection,
       _objectId(::ensureObjectId(basics::VelocyPackHelper::stringUInt64(info.get("objectId")))),
       _cf(cf),
       _cache(nullptr),
-      _cachePresent(false),
       _cacheEnabled(useCache && !collection.system() && CacheManagerFeature::MANAGER != nullptr) {
   TRI_ASSERT(cf != nullptr && cf != RocksDBColumnFamily::definitions());
 
@@ -147,7 +145,7 @@ void RocksDBIndex::load() {
 void RocksDBIndex::unload() {
   if (useCache()) {
     destroyCache();
-    TRI_ASSERT(!_cachePresent);
+    TRI_ASSERT(_cache.get() == nullptr);
   }
 }
 
@@ -163,7 +161,7 @@ void RocksDBIndex::toVelocyPack(VPackBuilder& builder,
 }
 
 void RocksDBIndex::createCache() {
-  if (!_cacheEnabled || _cachePresent || _collection.isAStub() ||
+  if (!_cacheEnabled || _cache != nullptr || _collection.isAStub() ||
       ServerState::instance()->isCoordinator()) {
     // we leave this if we do not need the cache
     // or if cache already created
@@ -175,12 +173,11 @@ void RocksDBIndex::createCache() {
   TRI_ASSERT(CacheManagerFeature::MANAGER != nullptr);
   LOG_TOPIC("49e6c", DEBUG, Logger::CACHE) << "Creating index cache";
   _cache = CacheManagerFeature::MANAGER->createCache(cache::CacheType::Transactional);
-  _cachePresent = (_cache.get() != nullptr);
   TRI_ASSERT(_cacheEnabled);
 }
 
 void RocksDBIndex::destroyCache() {
-  if (!_cachePresent) {
+  if (!_cache) {
     return;
   }
   TRI_ASSERT(CacheManagerFeature::MANAGER != nullptr);
@@ -189,7 +186,6 @@ void RocksDBIndex::destroyCache() {
   LOG_TOPIC("b5d85", DEBUG, Logger::CACHE) << "Destroying index cache";
   CacheManagerFeature::MANAGER->destroyCache(_cache);
   _cache.reset();
-  _cachePresent = false;
 }
 
 Result RocksDBIndex::drop() {
@@ -204,13 +200,12 @@ Result RocksDBIndex::drop() {
                                    prefixSameAsStart, useRangeDelete);
 
   // Try to drop the cache as well.
-  if (_cachePresent) {
+  if (_cache) {
     try {
       TRI_ASSERT(CacheManagerFeature::MANAGER != nullptr);
       CacheManagerFeature::MANAGER->destroyCache(_cache);
       // Reset flag
       _cache.reset();
-      _cachePresent = false;
     } catch (...) {
     }
   }
@@ -236,7 +231,7 @@ void RocksDBIndex::afterTruncate(TRI_voc_tick_t) {
   if (_cacheEnabled) {
     destroyCache();
     createCache();
-    TRI_ASSERT(_cachePresent);
+    TRI_ASSERT(_cache.get() != nullptr);
   }
 }  
 

arangod/RocksDBEngine/RocksDBIndex.h

@@ -131,7 +131,7 @@ class RocksDBIndex : public Index {
                arangodb::velocypack::Slice const& info,
                rocksdb::ColumnFamilyHandle* cf, bool useCache);
 
-  inline bool useCache() const { return (_cacheEnabled && _cachePresent); }
+  inline bool useCache() const { return (_cacheEnabled && _cache); }
   void blackListKey(char const* data, std::size_t len);
   void blackListKey(arangodb::velocypack::StringRef& ref) {
     blackListKey(ref.data(), ref.size());
@@ -142,9 +142,6 @@ class RocksDBIndex : public Index {
   rocksdb::ColumnFamilyHandle* _cf;
 
   mutable std::shared_ptr<cache::Cache> _cache;
-  // we use this boolean for testing whether _cache is set.
-  // it's quicker than accessing the shared_ptr each time
-  bool _cachePresent;
   bool _cacheEnabled;
 };
 }  // namespace arangodb