From fc00054c01c789119d37b425b287b48e21c7dbef Mon Sep 17 00:00:00 2001 From: Miguel Portilla Date: Wed, 8 Apr 2020 11:49:28 -0400 Subject: [PATCH] Improve online delete backend locking --- src/ripple/app/misc/SHAMapStoreImp.cpp | 23 ++--- src/ripple/nodestore/DatabaseRotating.h | 12 +-- .../nodestore/impl/DatabaseRotatingImp.cpp | 98 ++++++++++++++++--- .../nodestore/impl/DatabaseRotatingImp.h | 62 +++--------- 4 files changed, 108 insertions(+), 87 deletions(-) diff --git a/src/ripple/app/misc/SHAMapStoreImp.cpp b/src/ripple/app/misc/SHAMapStoreImp.cpp index 5c2644e6353..ce948ea6378 100644 --- a/src/ripple/app/misc/SHAMapStoreImp.cpp +++ b/src/ripple/app/misc/SHAMapStoreImp.cpp @@ -446,23 +446,16 @@ SHAMapStoreImp::run() ; } - std::string nextArchiveDir = - dbRotating_->getWritableBackend()->getName(); lastRotated_ = validatedSeq; - std::shared_ptr oldBackend; - { - std::lock_guard lock (dbRotating_->peekMutex()); - - state_db_.setState (SavedState {newBackend->getName(), - nextArchiveDir, lastRotated_}); - clearCaches (validatedSeq); - oldBackend = dbRotating_->rotateBackends( - std::move(newBackend), - lock); - } - JLOG(journal_.warn()) << "finished rotation " << validatedSeq; + state_db_.setState(SavedState { + newBackend->getName(), + dbRotating_->getName(), + lastRotated_}); - oldBackend->setDeletePath(); + clearCaches(validatedSeq); + dbRotating_->rotateBackends(std::move(newBackend)); + + JLOG(journal_.warn()) << "finished rotation " << validatedSeq; } } } diff --git a/src/ripple/nodestore/DatabaseRotating.h b/src/ripple/nodestore/DatabaseRotating.h index b44c6849c23..9e96dba8eb1 100644 --- a/src/ripple/nodestore/DatabaseRotating.h +++ b/src/ripple/nodestore/DatabaseRotating.h @@ -47,17 +47,9 @@ class DatabaseRotating : public Database TaggedCache const& getPositiveCache() = 0; - virtual std::mutex& peekMutex() const = 0; - - virtual - std::shared_ptr const& - getWritableBackend() const = 0; - virtual - std::shared_ptr - rotateBackends( - std::shared_ptr newBackend, - std::lock_guard const&) = 0; + void + rotateBackends(std::shared_ptr newBackend) = 0; }; } diff --git a/src/ripple/nodestore/impl/DatabaseRotatingImp.cpp b/src/ripple/nodestore/impl/DatabaseRotatingImp.cpp index 00a9365cc73..35bf9addf2f 100644 --- a/src/ripple/nodestore/impl/DatabaseRotatingImp.cpp +++ b/src/ripple/nodestore/impl/DatabaseRotatingImp.cpp @@ -48,15 +48,54 @@ DatabaseRotatingImp::DatabaseRotatingImp( setParent(parent); } -std::shared_ptr -DatabaseRotatingImp::rotateBackends( - std::shared_ptr newBackend, - std::lock_guard const&) +void +DatabaseRotatingImp::rotateBackends(std::shared_ptr newBackend) { - auto oldBackend {std::move(archiveBackend_)}; + std::lock_guard lock(mutex_); + archiveBackend_->setDeletePath(); archiveBackend_ = std::move(writableBackend_); writableBackend_ = std::move(newBackend); - return oldBackend; +} + +std::string +DatabaseRotatingImp::getName() const +{ + std::lock_guard lock(mutex_); + return writableBackend_->getName(); +} + +std::int32_t +DatabaseRotatingImp::getWriteLoad() const +{ + std::lock_guard lock(mutex_); + return writableBackend_->getWriteLoad(); +} + +void +DatabaseRotatingImp::import(Database& source) +{ + auto const backend = [&] { + std::lock_guard lock(mutex_); + return writableBackend_; + }(); + + importInternal(*backend, source); +} + +bool +DatabaseRotatingImp::storeLedger(std::shared_ptr const& srcLedger) +{ + auto const backend = [&] { + std::lock_guard lock(mutex_); + return writableBackend_; + }(); + + return Database::storeLedger( + *srcLedger, + backend, + pCache_, + nCache_, + nullptr); } void @@ -65,7 +104,13 @@ DatabaseRotatingImp::store(NodeObjectType type, Blob&& data, { auto nObj = NodeObject::createObject(type, std::move(data), hash); pCache_->canonicalize_replace_cache(hash, nObj); - getWritableBackend()->store(nObj); + + auto const backend = [&] { + std::lock_guard lock(mutex_); + return writableBackend_; + }(); + backend->store(nObj); + nCache_->erase(hash); storeStats(nObj->getData().size()); } @@ -78,6 +123,7 @@ DatabaseRotatingImp::asyncFetch(uint256 const& hash, object = pCache_->fetch(hash); if (object || nCache_->touch_if_exists(hash)) return true; + // Otherwise post a read Database::asyncFetch(hash, seq, pCache_, nCache_); return false; @@ -102,19 +148,49 @@ DatabaseRotatingImp::sweep() std::shared_ptr DatabaseRotatingImp::fetchFrom(uint256 const& hash, std::uint32_t seq) { - Backends b = getBackends(); - auto nObj = fetchInternal(hash, b.writableBackend); + auto backends = [&] { + std::lock_guard lock(mutex_); + return std::make_pair(writableBackend_, archiveBackend_); + }(); + + // Try to fetch from the writable backend + auto nObj = fetchInternal(hash, backends.first); if (! nObj) { - nObj = fetchInternal(hash, b.archiveBackend); + // Otherwise try to fetch from the archive backend + nObj = fetchInternal(hash, backends.second); if (nObj) { - getWritableBackend()->store(nObj); + { + // Refresh the writable backend pointer + std::lock_guard lock(mutex_); + backends.first = writableBackend_; + } + + // Update writable backend with data from the archive backend + backends.first->store(nObj); nCache_->erase(hash); } } return nObj; } +void +DatabaseRotatingImp::for_each( + std::function )> f) +{ + auto const backends = [&] { + std::lock_guard lock(mutex_); + return std::make_pair(writableBackend_, archiveBackend_); + }(); + + // Iterate the writable backend + backends.first->for_each(f); + + // Iterate the archive backend + backends.second->for_each(f); +} + + } // NodeStore } // ripple diff --git a/src/ripple/nodestore/impl/DatabaseRotatingImp.h b/src/ripple/nodestore/impl/DatabaseRotatingImp.h index 4cdf6396f87..a44e5244d0b 100644 --- a/src/ripple/nodestore/impl/DatabaseRotatingImp.h +++ b/src/ripple/nodestore/impl/DatabaseRotatingImp.h @@ -48,37 +48,17 @@ class DatabaseRotatingImp : public DatabaseRotating stopThreads(); } - std::shared_ptr const& - getWritableBackend() const override - { - std::lock_guard lock (rotateMutex_); - return writableBackend_; - } - - std::shared_ptr - rotateBackends( - std::shared_ptr newBackend, - std::lock_guard const&) override; - - std::mutex& peekMutex() const override - { - return rotateMutex_; - } + void + rotateBackends(std::shared_ptr newBackend) override; - std::string getName() const override - { - return getWritableBackend()->getName(); - } + std::string + getName() const override; - std::int32_t getWriteLoad() const override - { - return getWritableBackend()->getWriteLoad(); - } + std::int32_t + getWriteLoad() const override; - void import (Database& source) override - { - importInternal (*getWritableBackend(), source); - } + void + import(Database& source) override; void store(NodeObjectType type, Blob&& data, uint256 const& hash, std::uint32_t seq) override; @@ -94,11 +74,7 @@ class DatabaseRotatingImp : public DatabaseRotating std::shared_ptr& object) override; bool - storeLedger(std::shared_ptr const& srcLedger) override - { - return Database::storeLedger( - *srcLedger, getWritableBackend(), pCache_, nCache_, nullptr); - } + storeLedger(std::shared_ptr const& srcLedger) override; int getDesiredAsyncReadCount(std::uint32_t seq) override @@ -130,29 +106,13 @@ class DatabaseRotatingImp : public DatabaseRotating std::shared_ptr writableBackend_; std::shared_ptr archiveBackend_; - mutable std::mutex rotateMutex_; - - struct Backends { - std::shared_ptr const& writableBackend; - std::shared_ptr const& archiveBackend; - }; - - Backends getBackends() const - { - std::lock_guard lock (rotateMutex_); - return Backends {writableBackend_, archiveBackend_}; - } + mutable std::mutex mutex_; std::shared_ptr fetchFrom( uint256 const& hash, std::uint32_t seq) override; void - for_each(std::function )> f) override - { - Backends b = getBackends(); - b.archiveBackend->for_each(f); - b.writableBackend->for_each(f); - } + for_each(std::function )> f) override; }; }