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

@@ -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 <bool replace>
+public:
  bool
  canonicalize(
  const key_type& key,
- std::conditional_t<
- replace,
- std::shared_ptr<T> const,
- std::shared_ptr<T>>& data)
+ std::shared_ptr<T>& data,
+ std::function<bool(std::shared_ptr<T> 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<T> const& data)
  {
- return canonicalize<true>(key, data);
+ return canonicalize(
+ key,
+ const_cast<std::shared_ptr<T>&>(data),
+ [](std::shared_ptr<T> const&) { return true; });
  }
 
  bool
  canonicalize_replace_client(const key_type& key, std::shared_ptr<T>& data)
  {
- return canonicalize<false>(key, data);
+ return canonicalize(
+ key, data, [](std::shared_ptr<T> const&) { return false; });
  }
 
  std::shared_ptr<T>

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,

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.

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<NodeObject> const& n) {
+ return n->getType() == hotDUMMY;
+ });
+ }
+}
+
+void
+DatabaseNodeImp::asyncFetch(
+ uint256 const& hash,
+ std::uint32_t ledgerSeq,
+ std::function<void(std::shared_ptr<NodeObject> const&)>&& callback)
+{
+ if (cache_)
+ {
+ std::shared_ptr<NodeObject> 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<uint256> 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<uint256> 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<uint256> 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 =

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<std::shared_ptr<NodeObject>>
  fetchBatch(std::vector<uint256> const& hashes);
 
+ void
+ asyncFetch(
+ uint256 const& hash,
+ std::uint32_t ledgerSeq,
+ std::function<void(std::shared_ptr<NodeObject> const&)>&& callback)
+ override;
+
  bool
  storeLedger(std::shared_ptr<Ledger const> const& srcLedger) override
  {