From dc778536edc1fa12e7fbb7b7c6760e16c4bb8e57 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Sun, 19 Dec 2021 23:57:45 -0800 Subject: [PATCH] Provide sensible default values for nodestore cache: The nodestore includes a built-in cache to reduce the disk I/O load but, by default, this cache was not initialized unless it was explicitly configured by the server operator. This commit introduces sensible defaults based on the server's configured node size. It remains possible to completely disable the cache if desired by explicitly configuring it the cache size and age parameters to 0: [node_db] ... cache_size = 0 cache_age = 0 --- src/ripple/app/misc/SHAMapStoreImp.cpp | 23 ++++++++++-- src/ripple/nodestore/impl/DatabaseNodeImp.cpp | 35 +++++++++++-------- src/ripple/nodestore/impl/DatabaseNodeImp.h | 14 ++++---- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index 42a6bc188..3930e04bf 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -168,7 +168,23 @@ SHAMapStoreImp::SHAMapStoreImp( std::unique_ptr SHAMapStoreImp::makeNodeStore(std::int32_t readThreads) { + auto nscfg = app_.config().section(ConfigSection::nodeDatabase()); + + // Provide default values: + if (!nscfg.exists("cache_size")) + nscfg.set( + "cache_size", + std::to_string(app_.config().getValueFor( + SizedItem::treeCacheSize, std::nullopt))); + + if (!nscfg.exists("cache_age")) + nscfg.set( + "cache_age", + std::to_string(app_.config().getValueFor( + SizedItem::treeCacheAge, std::nullopt))); + std::unique_ptr db; + if (deleteInterval_) { if (app_.config().reporting()) @@ -187,13 +203,14 @@ SHAMapStoreImp::makeNodeStore(std::int32_t readThreads) state_db_.setState(state); } - // Create NodeStore with two backends to allow online deletion of data + // Create NodeStore with two backends to allow online deletion of + // data auto dbr = std::make_unique( scheduler_, readThreads, std::move(writableBackend), std::move(archiveBackend), - app_.config().section(ConfigSection::nodeDatabase()), + nscfg, app_.logs().journal(nodeStoreName_)); fdRequired_ += dbr->fdRequired(); dbRotating_ = dbr.get(); @@ -206,7 +223,7 @@ SHAMapStoreImp::makeNodeStore(std::int32_t readThreads) app_.config().getValueFor(SizedItem::burstSize, std::nullopt)), scheduler_, readThreads, - app_.config().section(ConfigSection::nodeDatabase()), + nscfg, app_.logs().journal(nodeStoreName_)); fdRequired_ += db->fdRequired(); } diff --git a/src/ripple/nodestore/impl/DatabaseNodeImp.cpp b/src/ripple/nodestore/impl/DatabaseNodeImp.cpp index b5369f56d..5de22ccbb 100644 --- a/src/ripple/nodestore/impl/DatabaseNodeImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseNodeImp.cpp @@ -31,9 +31,9 @@ DatabaseNodeImp::store( uint256 const& hash, std::uint32_t) { - auto nObj = NodeObject::createObject(type, std::move(data), hash); - backend_->store(nObj); - storeStats(1, nObj->getData().size()); + storeStats(1, data.size()); + + backend_->store(NodeObject::createObject(type, std::move(data), hash)); } void @@ -50,12 +50,14 @@ DatabaseNodeImp::fetchNodeObject( FetchReport& fetchReport, bool duplicate) { - std::shared_ptr nodeObject{ - cache_ ? cache_->fetch(hash) : nullptr}; + std::shared_ptr nodeObject = + cache_ ? cache_->fetch(hash) : nullptr; + if (!nodeObject) { - JLOG(j_.trace()) - << "DatabaseNodeImp::fetchNodeObject - record not in cache"; + JLOG(j_.trace()) << "fetchNodeObject " << hash << ": record not " + << (cache_ ? "cached" : "found"); + Status status; try @@ -64,7 +66,9 @@ DatabaseNodeImp::fetchNodeObject( } catch (std::exception const& e) { - JLOG(j_.fatal()) << "Exception, " << e.what(); + JLOG(j_.fatal()) + << "fetchNodeObject " << hash + << ": Exception fetching from backend: " << e.what(); Rethrow(); } @@ -77,17 +81,20 @@ DatabaseNodeImp::fetchNodeObject( case notFound: break; case dataCorrupt: - JLOG(j_.fatal()) << "Corrupt NodeObject #" << hash; + JLOG(j_.fatal()) << "fetchNodeObject " << hash + << ": nodestore data is corrupted"; break; default: - JLOG(j_.warn()) << "Unknown status=" << status; + JLOG(j_.warn()) + << "fetchNodeObject " << hash + << ": backend returns unknown result " << status; break; } } else { - JLOG(j_.trace()) - << "DatabaseNodeImp::fetchNodeObject - record in cache"; + JLOG(j_.trace()) << "fetchNodeObject " << hash + << ": record found in cache"; } if (nodeObject) @@ -126,7 +133,7 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) } } - JLOG(j_.debug()) << "DatabaseNodeImp::fetchBatch - cache hits = " + JLOG(j_.debug()) << "fetchBatch - cache hits = " << (hashes.size() - cacheMisses.size()) << " - cache misses = " << cacheMisses.size(); auto dbResults = backend_->fetchBatch(cacheMisses).first; @@ -147,7 +154,7 @@ DatabaseNodeImp::fetchBatch(std::vector const& hashes) else { JLOG(j_.error()) - << "DatabaseNodeImp::fetchBatch - " + << "fetchBatch - " << "record not found in db or cache. hash = " << strHex(hash); } } diff --git a/src/ripple/nodestore/impl/DatabaseNodeImp.h b/src/ripple/nodestore/impl/DatabaseNodeImp.h index 3eaecd897..478b3cf66 100644 --- a/src/ripple/nodestore/impl/DatabaseNodeImp.h +++ b/src/ripple/nodestore/impl/DatabaseNodeImp.h @@ -45,6 +45,7 @@ public: , backend_(std::move(backend)) { std::optional cacheSize, cacheAge; + if (config.exists("cache_size")) { cacheSize = get(config, "cache_size"); @@ -54,6 +55,7 @@ public: "Specified negative value for cache_size"); } } + if (config.exists("cache_age")) { cacheAge = get(config, "cache_age"); @@ -63,19 +65,17 @@ public: "Specified negative value for cache_age"); } } - if (cacheSize || cacheAge) + + if (cacheSize != 0 || cacheAge != 0) { - if (!cacheSize || *cacheSize == 0) - cacheSize = 16384; - if (!cacheAge || *cacheAge == 0) - cacheAge = 5; cache_ = std::make_shared>( "DatabaseNodeImp", - cacheSize.value(), - std::chrono::minutes{cacheAge.value()}, + cacheSize.value_or(0), + std::chrono::minutes(cacheAge.value_or(0)), stopwatch(), j); } + assert(backend_); }