From 3eb8aa8b80bd818f04c99cee2cfc243192709667 Mon Sep 17 00:00:00 2001 From: JoelKatz Date: Wed, 29 Dec 2021 23:10:22 -0800 Subject: [PATCH] Negative cache support for node store Adds support to TaggedCache to support smart replacement (Needed to avoid race conditions with negative caching.) Create a "hotDUMMY" object that represents the absence of an object. Allow DatabaseNodeImp::asyncFetch to complete immediately if object is in cache (positive or negative). Fix a bug in asyncFetch where the object returned may not be the correct canonical version because we stash the object in the results array before we canonicalize it. --- src/ripple/basics/TaggedCache.h | 24 ++++---- src/ripple/nodestore/Database.h | 2 +- src/ripple/nodestore/NodeObject.h | 3 +- src/ripple/nodestore/impl/DatabaseNodeImp.cpp | 59 +++++++++++++++++-- src/ripple/nodestore/impl/DatabaseNodeImp.h | 8 +++ 5 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/ripple/basics/TaggedCache.h b/src/ripple/basics/TaggedCache.h index 45f069bd97b..548d21dc78e 100644 --- a/src/ripple/basics/TaggedCache.h +++ b/src/ripple/basics/TaggedCache.h @@ -300,19 +300,16 @@ class TaggedCache @param key The key corresponding to the object @param data A shared pointer to the data corresponding to the object. - @param replace `true` if `data` is the up to date version of the object. + @param replace Function that decides if cache should be replaced @return `true` If the key already existed. */ -private: - template +public: bool canonicalize( const key_type& key, - std::conditional_t< - replace, - std::shared_ptr const, - std::shared_ptr>& data) + std::shared_ptr& data, + std::function const&)>&& replace) { // Return canonical value, store if needed, refresh in cache // Return values: true=we had the data already @@ -335,7 +332,7 @@ class TaggedCache if (entry.isCached()) { - if constexpr (replace) + if (replace(entry.ptr)) { entry.ptr = data; entry.weak_ptr = data; @@ -352,7 +349,7 @@ class TaggedCache if (cachedData) { - if constexpr (replace) + if (replace(entry.ptr)) { entry.ptr = data; entry.weak_ptr = data; @@ -374,19 +371,22 @@ class TaggedCache return false; } -public: bool canonicalize_replace_cache( const key_type& key, std::shared_ptr const& data) { - return canonicalize(key, data); + return canonicalize( + key, + const_cast&>(data), + [](std::shared_ptr const&) { return true; }); } bool canonicalize_replace_client(const key_type& key, std::shared_ptr& data) { - return canonicalize(key, data); + return canonicalize( + key, data, [](std::shared_ptr const&) { return false; }); } std::shared_ptr diff --git a/src/ripple/nodestore/Database.h b/src/ripple/nodestore/Database.h index f9e8c2418bf..6cea6d46ced 100644 --- a/src/ripple/nodestore/Database.h +++ b/src/ripple/nodestore/Database.h @@ -156,7 +156,7 @@ class Database object is stored, used by the shard store. @param callback Callback function when read completes */ - void + virtual void asyncFetch( uint256 const& hash, std::uint32_t ledgerSeq, diff --git a/src/ripple/nodestore/NodeObject.h b/src/ripple/nodestore/NodeObject.h index 2bd73d8dee5..a94e689b34b 100644 --- a/src/ripple/nodestore/NodeObject.h +++ b/src/ripple/nodestore/NodeObject.h @@ -33,7 +33,8 @@ enum NodeObjectType : std::uint32_t { hotUNKNOWN = 0, hotLEDGER = 1, hotACCOUNT_NODE = 3, - hotTRANSACTION_NODE = 4 + hotTRANSACTION_NODE = 4, + hotDUMMY = 512 // an invalid or missing object }; /** A simple object that the Ledger uses to store entries. diff --git a/src/ripple/nodestore/impl/DatabaseNodeImp.cpp b/src/ripple/nodestore/impl/DatabaseNodeImp.cpp index 5de22ccbbb4..9ef878bf3be 100644 --- a/src/ripple/nodestore/impl/DatabaseNodeImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseNodeImp.cpp @@ -33,7 +33,34 @@ DatabaseNodeImp::store( { storeStats(1, data.size()); - backend_->store(NodeObject::createObject(type, std::move(data), hash)); + auto obj = NodeObject::createObject(type, std::move(data), hash); + backend_->store(obj); + if (cache_) + { + // After the store, replace a negative cache entry if there is one + cache_->canonicalize( + hash, obj, [](std::shared_ptr const& n) { + return n->getType() == hotDUMMY; + }); + } +} + +void +DatabaseNodeImp::asyncFetch( + uint256 const& hash, + std::uint32_t ledgerSeq, + std::function const&)>&& callback) +{ + if (cache_) + { + std::shared_ptr obj = cache_->fetch(hash); + if (obj) + { + callback(obj->getType() == hotDUMMY ? nullptr : obj); + return; + } + } + Database::asyncFetch(hash, ledgerSeq, std::move(callback)); } void @@ -75,8 +102,19 @@ DatabaseNodeImp::fetchNodeObject( switch (status) { case ok: - if (nodeObject && cache_) - cache_->canonicalize_replace_client(hash, nodeObject); + if (cache_) + { + if (nodeObject) + cache_->canonicalize_replace_client(hash, nodeObject); + else + { + auto notFound = + NodeObject::createObject(hotDUMMY, {}, hash); + cache_->canonicalize_replace_client(hash, notFound); + if (notFound->getType() != hotDUMMY) + nodeObject = notFound; + } + } break; case notFound: break; @@ -95,6 +133,8 @@ DatabaseNodeImp::fetchNodeObject( { JLOG(j_.trace()) << "fetchNodeObject " << hash << ": record found in cache"; + if (nodeObject->getType() == hotDUMMY) + nodeObject.reset(); } if (nodeObject) @@ -127,7 +167,7 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) } else { - results[i] = nObj; + results[i] = nObj->getType() == hotDUMMY ? nullptr : nObj; // It was in the cache. ++hits; } @@ -140,9 +180,8 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) for (size_t i = 0; i < dbResults.size(); ++i) { - auto nObj = dbResults[i]; + auto nObj = std::move(dbResults[i]); size_t index = indexMap[cacheMisses[i]]; - results[index] = nObj; auto const& hash = hashes[index]; if (nObj) @@ -156,7 +195,15 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) JLOG(j_.error()) << "fetchBatch - " << "record not found in db or cache. hash = " << strHex(hash); + if (cache_) + { + auto notFound = NodeObject::createObject(hotDUMMY, {}, hash); + cache_->canonicalize_replace_client(hash, notFound); + if (notFound->getType() != hotDUMMY) + nObj = std::move(notFound); + } } + results[index] = std::move(nObj); } auto fetchDurationUs = diff --git a/src/ripple/nodestore/impl/DatabaseNodeImp.h b/src/ripple/nodestore/impl/DatabaseNodeImp.h index 478b3cf6660..452bd8d27fe 100644 --- a/src/ripple/nodestore/impl/DatabaseNodeImp.h +++ b/src/ripple/nodestore/impl/DatabaseNodeImp.h @@ -111,6 +111,7 @@ class DatabaseNodeImp : public Database // only one database return true; } + void sync() override { @@ -120,6 +121,13 @@ class DatabaseNodeImp : public Database std::vector> fetchBatch(std::vector const& hashes); + void + asyncFetch( + uint256 const& hash, + std::uint32_t ledgerSeq, + std::function const&)>&& callback) + override; + bool storeLedger(std::shared_ptr const& srcLedger) override {