From 0f31920ab8e8bcd53951fcb6debd43305cd6ea03 Mon Sep 17 00:00:00 2001 From: Garand Tyson Date: Sun, 29 Dec 2024 20:18:20 -0800 Subject: [PATCH] Delete restored keys from hot archive --- src/bucket/test/BucketTestUtils.cpp | 8 +- src/ledger/LedgerManagerImpl.cpp | 7 +- src/ledger/LedgerTxn.cpp | 87 +++++++++++++++++--- src/ledger/LedgerTxn.h | 27 ++++-- src/ledger/LedgerTxnImpl.h | 21 ++++- src/ledger/test/InMemoryLedgerTxn.cpp | 15 +++- src/ledger/test/InMemoryLedgerTxn.h | 2 + src/ledger/test/InMemoryLedgerTxnRoot.cpp | 5 +- src/ledger/test/InMemoryLedgerTxnRoot.h | 1 + src/ledger/test/LedgerTxnTests.cpp | 60 ++++++++++++++ src/transactions/RestoreFootprintOpFrame.cpp | 6 +- 11 files changed, 212 insertions(+), 27 deletions(-) diff --git a/src/bucket/test/BucketTestUtils.cpp b/src/bucket/test/BucketTestUtils.cpp index ea6d0f351f..79ba3b019f 100644 --- a/src/bucket/test/BucketTestUtils.cpp +++ b/src/bucket/test/BucketTestUtils.cpp @@ -235,15 +235,19 @@ LedgerManagerForBucketTests::transferLedgerEntriesToBucketList( ltxEvictions, lh.ledgerSeq, keys, initialLedgerVers, mApp.getLedgerManager() .getSorobanNetworkConfigForApply()); - +#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION if (protocolVersionStartsFrom( initialLedgerVers, BucketBase:: FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION)) { + std::vector restoredKeys; + ltx.getRestoredHotArchiveKeys(restoredKeys); mApp.getBucketManager().addHotArchiveBatch( - mApp, lh, evictedState.archivedEntries, {}, {}); + mApp, lh, evictedState.archivedEntries, restoredKeys, + {}); } +#endif if (ledgerCloseMeta) { ledgerCloseMeta->populateEvictedEntries(evictedState); diff --git a/src/ledger/LedgerManagerImpl.cpp b/src/ledger/LedgerManagerImpl.cpp index f5ea93dbff..c39eafa065 100644 --- a/src/ledger/LedgerManagerImpl.cpp +++ b/src/ledger/LedgerManagerImpl.cpp @@ -45,6 +45,7 @@ #include +#include "xdr/Stellar-ledger-entries.h" #include "xdr/Stellar-ledger.h" #include "xdr/Stellar-transaction.h" #include "xdrpp/types.h" @@ -1787,13 +1788,17 @@ LedgerManagerImpl::transferLedgerEntriesToBucketList( ltxEvictions, lh.ledgerSeq, keys, initialLedgerVers, *mSorobanNetworkConfigForApply); +#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION if (protocolVersionStartsFrom( initialLedgerVers, BucketBase::FIRST_PROTOCOL_SUPPORTING_PERSISTENT_EVICTION)) { + std::vector restoredKeys; + ltx.getRestoredHotArchiveKeys(restoredKeys); mApp.getBucketManager().addHotArchiveBatch( - mApp, lh, evictedState.archivedEntries, {}, {}); + mApp, lh, evictedState.archivedEntries, restoredKeys, {}); } +#endif if (ledgerCloseMeta) { diff --git a/src/ledger/LedgerTxn.cpp b/src/ledger/LedgerTxn.cpp index c2a98ab52c..4a257cfc2e 100644 --- a/src/ledger/LedgerTxn.cpp +++ b/src/ledger/LedgerTxn.cpp @@ -16,12 +16,14 @@ #include "main/Application.h" #include "transactions/TransactionUtils.h" #include "util/GlobalChecks.h" +#include "util/UnorderedSet.h" #include "util/types.h" #include "xdr/Stellar-ledger-entries.h" #include #include #include +#include namespace stellar { @@ -501,14 +503,17 @@ LedgerTxn::Impl::commit() noexcept maybeUpdateLastModifiedThenInvokeThenSeal([&](EntryMap const& entries) { // getEntryIterator has the strong exception safety guarantee // commitChild has the strong exception safety guarantee - mParent.commitChild(getEntryIterator(entries), mConsistency); + mParent.commitChild(getEntryIterator(entries), mRestoredHotArchiveKeys, + mConsistency); }); } void -LedgerTxn::commitChild(EntryIterator iter, LedgerTxnConsistency cons) noexcept +LedgerTxn::commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { - getImpl()->commitChild(std::move(iter), cons); + getImpl()->commitChild(std::move(iter), restoredHotArchiveKeys, cons); } static LedgerTxnConsistency @@ -526,8 +531,9 @@ joinConsistencyLevels(LedgerTxnConsistency c1, LedgerTxnConsistency c2) } void -LedgerTxn::Impl::commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept +LedgerTxn::Impl::commitChild( + EntryIterator iter, UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { // Assignment of xdrpp objects does not have the strong exception safety // guarantee, so use std::unique_ptr<...>::swap to achieve it @@ -632,6 +638,15 @@ LedgerTxn::Impl::commitChild(EntryIterator iter, printErrorAndAbort("unknown fatal error during commit to LedgerTxn"); } + for (auto const& key : restoredHotArchiveKeys) + { + auto [_, inserted] = mRestoredHotArchiveKeys.emplace(key); + if (!inserted) + { + printErrorAndAbort("restored hot archive entry already exists"); + } + } + // std::unique_ptr<...>::swap does not throw mHeader.swap(childHeader); mChild = nullptr; @@ -802,6 +817,37 @@ LedgerTxn::Impl::erase(InternalLedgerKey const& key) } } +void +LedgerTxn::removeFromHotArchive(LedgerKey const& key) +{ + getImpl()->removeFromHotArchive(key); +} + +void +LedgerTxn::Impl::removeFromHotArchive(LedgerKey const& key) +{ + throwIfSealed(); + throwIfChild(); + + if (!isPersistentEntry(key)) + { + throw std::runtime_error("Key type not supported in Hot Archive"); + } + + auto restoredEntry = mEntry.find(key); + if (restoredEntry == mEntry.end() || !restoredEntry->second.isInit()) + { + throw std::runtime_error( + "Restored entry must be created before Hot Archive removal"); + } + + auto [_, inserted] = mRestoredHotArchiveKeys.insert(key); + if (!inserted) + { + throw std::runtime_error("Key already removed from hot archive"); + } +} + void LedgerTxn::eraseWithoutLoading(InternalLedgerKey const& key) { @@ -1470,6 +1516,24 @@ LedgerTxn::Impl::getAllEntries(std::vector& initEntries, deadEntries.swap(resDead); } +void +LedgerTxn::getRestoredHotArchiveKeys( + std::vector& restoredHotArchiveKeys) +{ + getImpl()->getRestoredHotArchiveKeys(restoredHotArchiveKeys); +} + +void +LedgerTxn::Impl::getRestoredHotArchiveKeys( + std::vector& restoredHotArchiveKeys) +{ + restoredHotArchiveKeys.reserve(mRestoredHotArchiveKeys.size()); + for (auto const& key : mRestoredHotArchiveKeys) + { + restoredHotArchiveKeys.emplace_back(key); + } +} + LedgerKeySet LedgerTxn::getAllTTLKeysWithoutSealing() const { @@ -1957,6 +2021,7 @@ LedgerTxn::Impl::rollback() noexcept } mEntry.clear(); + mRestoredHotArchiveKeys.clear(); mMultiOrderBook.clear(); mActive.clear(); mActiveHeader.reset(); @@ -2559,10 +2624,11 @@ LedgerTxnRoot::Impl::throwIfChild() const } void -LedgerTxnRoot::commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept +LedgerTxnRoot::commitChild( + EntryIterator iter, UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { - mImpl->commitChild(std::move(iter), cons); + mImpl->commitChild(std::move(iter), restoredHotArchiveKeys, cons); } static void @@ -2618,8 +2684,9 @@ LedgerTxnRoot::Impl::bulkApply(BulkLedgerEntryChangeAccumulator& bleca, } void -LedgerTxnRoot::Impl::commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept +LedgerTxnRoot::Impl::commitChild( + EntryIterator iter, UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { ZoneScoped; diff --git a/src/ledger/LedgerTxn.h b/src/ledger/LedgerTxn.h index e1ca8f9b22..4e01278e76 100644 --- a/src/ledger/LedgerTxn.h +++ b/src/ledger/LedgerTxn.h @@ -420,8 +420,10 @@ class AbstractLedgerTxnParent // commitChild and rollbackChild are called by a child AbstractLedgerTxn // to trigger an atomic commit or an atomic rollback of the data stored in // the child. - virtual void commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept = 0; + virtual void + commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept = 0; virtual void rollbackChild() noexcept = 0; // getAllOffers, getBestOffer, and getOffersByAccountAndAsset are used to @@ -539,10 +541,10 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent virtual void commit() noexcept = 0; virtual void rollback() noexcept = 0; - // loadHeader, create, erase, load, and loadWithoutRecord provide the main - // interface to interact with data stored in the AbstractLedgerTxn. These - // functions only allow one instance of a particular data to be active at a - // time. + // loadHeader, create, erase, load, loadWithoutRecord, and + // removeFromHotArchive provide the main interface to interact with data + // stored in the AbstractLedgerTxn. These functions only allow one instance + // of a particular data to be active at a time. // - loadHeader // Loads the current LedgerHeader. Throws if there is already an active // LedgerTxnHeader. @@ -566,11 +568,17 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent // then it will still be recorded after calling loadWithoutRecord. // Throws if there is an active LedgerTxnEntry associated with this // key. + // - removeFromHotArchive: + // Indicates that an entry in the Hot Archive has been restored and must + // be removed from the Hot Archive. Prior to this call, the key must + // be created via a call to create. Throws if key has not been + // previously created in this LedgerTxn. // All of these functions throw if the AbstractLedgerTxn is sealed or if // the AbstractLedgerTxn has a child. virtual LedgerTxnHeader loadHeader() = 0; virtual LedgerTxnEntry create(InternalLedgerEntry const& entry) = 0; virtual void erase(InternalLedgerKey const& key) = 0; + virtual void removeFromHotArchive(LedgerKey const& key) = 0; virtual LedgerTxnEntry load(InternalLedgerKey const& key) = 0; virtual ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key) = 0; @@ -613,6 +621,8 @@ class AbstractLedgerTxn : public AbstractLedgerTxnParent virtual void getAllEntries(std::vector& initEntries, std::vector& liveEntries, std::vector& deadEntries) = 0; + virtual void + getRestoredHotArchiveKeys(std::vector& restoredKeys) = 0; // Returns all TTL keys that have been modified (create, update, and // delete), but does not cause the AbstractLedgerTxn or update last @@ -705,11 +715,13 @@ class LedgerTxn : public AbstractLedgerTxn void commit() noexcept override; void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, LedgerTxnConsistency cons) noexcept override; LedgerTxnEntry create(InternalLedgerEntry const& entry) override; void erase(InternalLedgerKey const& key) override; + void removeFromHotArchive(LedgerKey const& key) override; UnorderedMap getAllOffers() override; @@ -744,6 +756,8 @@ class LedgerTxn : public AbstractLedgerTxn void getAllEntries(std::vector& initEntries, std::vector& liveEntries, std::vector& deadEntries) override; + void + getRestoredHotArchiveKeys(std::vector& restoredKeys) override; LedgerKeySet getAllTTLKeysWithoutSealing() const override; std::shared_ptr @@ -831,6 +845,7 @@ class LedgerTxnRoot : public AbstractLedgerTxnParent void addChild(AbstractLedgerTxn& child, TransactionMode mode) override; void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, LedgerTxnConsistency cons) noexcept override; uint64_t countOffers(LedgerRange const& ledgers) const override; diff --git a/src/ledger/LedgerTxnImpl.h b/src/ledger/LedgerTxnImpl.h index 62afa831ef..316f89a0ca 100644 --- a/src/ledger/LedgerTxnImpl.h +++ b/src/ledger/LedgerTxnImpl.h @@ -8,6 +8,7 @@ #include "database/Database.h" #include "ledger/LedgerTxn.h" #include "util/RandomEvictionCache.h" +#include "util/UnorderedSet.h" #include #include #ifdef USE_POSTGRES @@ -94,6 +95,10 @@ class LedgerTxn::Impl std::unique_ptr mHeader; std::shared_ptr mActiveHeader; EntryMap mEntry; + + // Contains the set of keys for entries that currently exist in the hot + // archive and were restored. + UnorderedSet mRestoredHotArchiveKeys; UnorderedMap> mActive; bool const mShouldUpdateLastModified; bool mIsSealed; @@ -335,7 +340,9 @@ class LedgerTxn::Impl void commit() noexcept; - void commitChild(EntryIterator iter, LedgerTxnConsistency cons) noexcept; + void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept; // create has the basic exception safety guarantee. If it throws an // exception, then @@ -357,6 +364,12 @@ class LedgerTxn::Impl // - the entry cache may be, but is not guaranteed to be, cleared. void erase(InternalLedgerKey const& key); + // removeFromHotArchive has the basic exception safety guarantee. If it + // throws an exception, then + // - the prepared statement cache may be, but is not guaranteed to be, + // modified + void removeFromHotArchive(LedgerKey const& key); + // getAllOffers has the basic exception safety guarantee. If it throws an // exception, then // - the prepared statement cache may be, but is not guaranteed to be, @@ -430,6 +443,8 @@ class LedgerTxn::Impl void getAllEntries(std::vector& initEntries, std::vector& liveEntries, std::vector& deadEntries); + // getRestoredHotArchiveKeys has the strong exception safety guarantee + void getRestoredHotArchiveKeys(std::vector& restoredKeys); LedgerKeySet getAllTTLKeysWithoutSealing() const; @@ -706,7 +721,9 @@ class LedgerTxnRoot::Impl // addChild has the strong exception safety guarantee. void addChild(AbstractLedgerTxn& child, TransactionMode mode); - void commitChild(EntryIterator iter, LedgerTxnConsistency cons) noexcept; + void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept; // countOffers has the strong exception safety guarantee. uint64_t countOffers(LedgerRange const& ledgers) const; diff --git a/src/ledger/test/InMemoryLedgerTxn.cpp b/src/ledger/test/InMemoryLedgerTxn.cpp index 1b2dd26dfe..8c16b3dbd6 100644 --- a/src/ledger/test/InMemoryLedgerTxn.cpp +++ b/src/ledger/test/InMemoryLedgerTxn.cpp @@ -5,6 +5,7 @@ #include "ledger/test/InMemoryLedgerTxn.h" #include "ledger/LedgerTxn.h" #include "transactions/TransactionUtils.h" +#include "util/UnorderedSet.h" namespace stellar { @@ -186,8 +187,9 @@ InMemoryLedgerTxn::getFilteredEntryIterator(EntryIterator const& iter) } void -InMemoryLedgerTxn::commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept +InMemoryLedgerTxn::commitChild( + EntryIterator iter, UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { if (!mTransaction) { @@ -198,7 +200,7 @@ InMemoryLedgerTxn::commitChild(EntryIterator iter, auto filteredIter = getFilteredEntryIterator(iter); updateLedgerKeyMap(filteredIter); - LedgerTxn::commitChild(filteredIter, cons); + LedgerTxn::commitChild(filteredIter, restoredHotArchiveKeys, cons); mTransaction->commit(); mTransaction.reset(); } @@ -272,6 +274,13 @@ InMemoryLedgerTxn::erase(InternalLedgerKey const& key) throw std::runtime_error("called erase on InMemoryLedgerTxn"); } +void +InMemoryLedgerTxn::removeFromHotArchive(LedgerKey const& key) +{ + throw std::runtime_error( + "called removeFromHotArchive on InMemoryLedgerTxn"); +} + LedgerTxnEntry InMemoryLedgerTxn::load(InternalLedgerKey const& key) { diff --git a/src/ledger/test/InMemoryLedgerTxn.h b/src/ledger/test/InMemoryLedgerTxn.h index 9b49e8a890..5e44559d8d 100644 --- a/src/ledger/test/InMemoryLedgerTxn.h +++ b/src/ledger/test/InMemoryLedgerTxn.h @@ -103,6 +103,7 @@ class InMemoryLedgerTxn : public LedgerTxn void addChild(AbstractLedgerTxn& child, TransactionMode mode) override; void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, LedgerTxnConsistency cons) noexcept override; void rollbackChild() noexcept override; @@ -112,6 +113,7 @@ class InMemoryLedgerTxn : public LedgerTxn LedgerTxnEntry create(InternalLedgerEntry const& entry) override; void erase(InternalLedgerKey const& key) override; + void removeFromHotArchive(LedgerKey const& key) override; LedgerTxnEntry load(InternalLedgerKey const& key) override; ConstLedgerTxnEntry loadWithoutRecord(InternalLedgerKey const& key) override; diff --git a/src/ledger/test/InMemoryLedgerTxnRoot.cpp b/src/ledger/test/InMemoryLedgerTxnRoot.cpp index d2d2bd4ab1..262847bca9 100644 --- a/src/ledger/test/InMemoryLedgerTxnRoot.cpp +++ b/src/ledger/test/InMemoryLedgerTxnRoot.cpp @@ -33,8 +33,9 @@ InMemoryLedgerTxnRoot::addChild(AbstractLedgerTxn& child, TransactionMode mode) } void -InMemoryLedgerTxnRoot::commitChild(EntryIterator iter, - LedgerTxnConsistency cons) noexcept +InMemoryLedgerTxnRoot::commitChild( + EntryIterator iter, UnorderedSet const& restoredHotArchiveKeys, + LedgerTxnConsistency cons) noexcept { printErrorAndAbort("committing to stub InMemoryLedgerTxnRoot"); } diff --git a/src/ledger/test/InMemoryLedgerTxnRoot.h b/src/ledger/test/InMemoryLedgerTxnRoot.h index 16606faa6b..42a5beb9db 100644 --- a/src/ledger/test/InMemoryLedgerTxnRoot.h +++ b/src/ledger/test/InMemoryLedgerTxnRoot.h @@ -39,6 +39,7 @@ class InMemoryLedgerTxnRoot : public AbstractLedgerTxnParent ); void addChild(AbstractLedgerTxn& child, TransactionMode mode) override; void commitChild(EntryIterator iter, + UnorderedSet const& restoredHotArchiveKeys, LedgerTxnConsistency cons) noexcept override; void rollbackChild() noexcept override; diff --git a/src/ledger/test/LedgerTxnTests.cpp b/src/ledger/test/LedgerTxnTests.cpp index 11f0a2c9fd..b71a5b5368 100644 --- a/src/ledger/test/LedgerTxnTests.cpp +++ b/src/ledger/test/LedgerTxnTests.cpp @@ -238,6 +238,66 @@ TEST_CASE("LedgerTxn commit into LedgerTxn", "[ledgertxn]") validate(ltx1, {}); } } + + SECTION("restored hot archive keys") + { + auto randomEntries = + LedgerTestUtils::generateValidUniqueLedgerEntriesWithTypes( + {CONTRACT_CODE}, 2); + std::vector randomKeys = {LedgerEntryKey(randomEntries[0]), + LedgerEntryKey(randomEntries[1])}; + LedgerTxn ltx1(app->getLedgerTxnRoot()); + + SECTION("remove key before creation") + { + REQUIRE_THROWS(ltx1.removeFromHotArchive(randomKeys[0])); + } + + SECTION("commited to parent") + { + ltx1.create(randomEntries[0]); + ltx1.removeFromHotArchive(randomKeys[0]); + + SECTION("rollback") + { + { + LedgerTxn ltx2(ltx1); + ltx2.create(randomEntries[1]); + ltx2.removeFromHotArchive(randomKeys[1]); + } + + std::vector parentKeys; + ltx1.getRestoredHotArchiveKeys(parentKeys); + REQUIRE(parentKeys.size() == 1); + REQUIRE(parentKeys.front() == randomKeys[0]); + } + + SECTION("commit") + { + { + LedgerTxn ltx2(ltx1); + ltx2.create(randomEntries[1]); + ltx2.removeFromHotArchive(randomKeys[1]); + ltx2.commit(); + } + + std::vector parentKeys; + ltx1.getRestoredHotArchiveKeys(parentKeys); + REQUIRE(parentKeys.size() == 2); + REQUIRE(std::find(parentKeys.begin(), parentKeys.end(), + randomKeys[0]) != parentKeys.end()); + REQUIRE(std::find(parentKeys.begin(), parentKeys.end(), + randomKeys[1]) != parentKeys.end()); + } + } + + SECTION("commit same key twice") + { + ltx1.create(randomEntries[0]); + ltx1.removeFromHotArchive(randomKeys[0]); + REQUIRE_THROWS(ltx1.removeFromHotArchive(randomKeys[0])); + } + } } TEST_CASE("LedgerTxn rollback into LedgerTxn", "[ledgertxn]") diff --git a/src/transactions/RestoreFootprintOpFrame.cpp b/src/transactions/RestoreFootprintOpFrame.cpp index 7b0911fb89..9a5ddbb3d3 100644 --- a/src/transactions/RestoreFootprintOpFrame.cpp +++ b/src/transactions/RestoreFootprintOpFrame.cpp @@ -177,17 +177,21 @@ RestoreFootprintOpFrame::doApply( rustChange.new_size_bytes = entrySize; rustChange.new_live_until_ledger = restoredLiveUntilLedger; +#ifdef ENABLE_NEXT_PROTOCOL_VERSION_UNSAFE_FOR_PRODUCTION if (hotArchiveEntry) { - // TODO: Delete entries from Hot Archive ltx.create(hotArchiveEntry->archivedEntry()); LedgerEntry ttl; ttl.data.type(TTL); ttl.data.ttl().liveUntilLedgerSeq = restoredLiveUntilLedger; ttl.data.ttl().keyHash = getTTLKey(lk).ttl().keyHash; ltx.create(ttl); + + // Mark the entry as restored + ltx.removeFromHotArchive(lk); } else +#endif { // Entry exists if we get this this point due to the constTTLLtxe // loadWithoutRecord logic above.