From 00ed7c942436f02644a13169002b5123f4e2a116 Mon Sep 17 00:00:00 2001 From: Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> Date: Fri, 23 Aug 2024 14:43:02 -0400 Subject: [PATCH 1/6] Track latencies of certain code blocks, and log if they take too long --- src/ripple/app/consensus/RCLValidations.cpp | 9 +- src/ripple/app/ledger/impl/InboundLedgers.cpp | 128 ++++++++++-------- src/ripple/app/main/Application.cpp | 2 +- src/ripple/app/main/Main.cpp | 2 +- src/ripple/app/rdb/Download.h | 4 +- src/ripple/app/rdb/ShardArchive.h | 6 +- src/ripple/app/rdb/UnitaryShard.h | 12 +- src/ripple/app/rdb/Vacuum.h | 3 +- src/ripple/app/rdb/Wallet.h | 9 +- src/ripple/app/rdb/backend/detail/Node.h | 4 +- src/ripple/app/rdb/backend/detail/Shard.h | 4 +- .../app/rdb/backend/detail/impl/Node.cpp | 7 +- .../app/rdb/backend/detail/impl/Shard.cpp | 13 +- .../app/rdb/backend/impl/SQLiteDatabase.cpp | 4 +- src/ripple/app/rdb/impl/Download.cpp | 5 +- src/ripple/app/rdb/impl/ShardArchive.cpp | 7 +- src/ripple/app/rdb/impl/UnitaryShard.cpp | 20 +-- src/ripple/app/rdb/impl/Vacuum.cpp | 4 +- src/ripple/app/rdb/impl/Wallet.cpp | 11 +- src/ripple/basics/PerfLog.h | 24 ++++ src/ripple/core/Config.h | 6 + src/ripple/core/DatabaseCon.h | 38 ++++-- src/ripple/net/DatabaseBody.h | 5 +- src/ripple/net/DatabaseDownloader.h | 3 +- src/ripple/net/HTTPDownloader.h | 3 +- src/ripple/net/impl/DatabaseBody.ipp | 5 +- src/ripple/net/impl/DatabaseDownloader.cpp | 5 +- src/ripple/net/impl/HTTPDownloader.cpp | 2 +- src/ripple/nodestore/impl/Shard.cpp | 8 +- src/ripple/overlay/impl/PeerImp.cpp | 13 +- src/ripple/rpc/impl/ShardArchiveHandler.cpp | 4 +- src/test/app/Manifest_test.cpp | 2 +- 32 files changed, 246 insertions(+), 126 deletions(-) diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index ab9391385dd..6b626569e69 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -126,7 +127,13 @@ RCLValidationsAdaptor::now() const std::optional RCLValidationsAdaptor::acquire(LedgerHash const& hash) { - auto ledger = app_.getLedgerMaster().getLedgerByHash(hash); + using namespace std::chrono_literals; + auto ledger = perf::measureDurationAndLog( + [&]() { return app_.getLedgerMaster().getLedgerByHash(hash); }, + "getLedgerByHash", + 10ms, + j_); + if (!ledger) { JLOG(j_.debug()) diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index 0bff434edbc..b9b8b9fcfd2 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -69,76 +70,83 @@ class InboundLedgersImp : public InboundLedgers std::uint32_t seq, InboundLedger::Reason reason) override { - assert(hash.isNonZero()); - assert( - reason != InboundLedger::Reason::SHARD || - (seq != 0 && app_.getShardStore())); - - // probably not the right rule - if (app_.getOPs().isNeedNetworkLedger() && - (reason != InboundLedger::Reason::GENERIC) && - (reason != InboundLedger::Reason::CONSENSUS)) - return {}; - - bool isNew = true; - std::shared_ptr inbound; - { - ScopedLockType sl(mLock); - if (stopping_) - { + auto doAcquire = [&, seq, reason]() -> std::shared_ptr { + assert(hash.isNonZero()); + assert( + reason != InboundLedger::Reason::SHARD || + (seq != 0 && app_.getShardStore())); + + // probably not the right rule + if (app_.getOPs().isNeedNetworkLedger() && + (reason != InboundLedger::Reason::GENERIC) && + (reason != InboundLedger::Reason::CONSENSUS)) return {}; - } - auto it = mLedgers.find(hash); - if (it != mLedgers.end()) + bool isNew = true; + std::shared_ptr inbound; { - isNew = false; - inbound = it->second; - } - else - { - inbound = std::make_shared( - app_, - hash, - seq, - reason, - std::ref(m_clock), - mPeerSetBuilder->build()); - mLedgers.emplace(hash, inbound); - inbound->init(sl); - ++mCounter; + ScopedLockType sl(mLock); + if (stopping_) + { + return {}; + } + + auto it = mLedgers.find(hash); + if (it != mLedgers.end()) + { + isNew = false; + inbound = it->second; + } + else + { + inbound = std::make_shared( + app_, + hash, + seq, + reason, + std::ref(m_clock), + mPeerSetBuilder->build()); + mLedgers.emplace(hash, inbound); + inbound->init(sl); + ++mCounter; + } } - } - if (inbound->isFailed()) - return {}; + if (inbound->isFailed()) + return {}; - if (!isNew) - inbound->update(seq); + if (!isNew) + inbound->update(seq); - if (!inbound->isComplete()) - return {}; + if (!inbound->isComplete()) + return {}; - if (reason == InboundLedger::Reason::HISTORY) - { - if (inbound->getLedger()->stateMap().family().isShardBacked()) - app_.getNodeStore().storeLedger(inbound->getLedger()); - } - else if (reason == InboundLedger::Reason::SHARD) - { - auto shardStore = app_.getShardStore(); - if (!shardStore) + if (reason == InboundLedger::Reason::HISTORY) { - JLOG(j_.error()) - << "Acquiring shard with no shard store available"; - return {}; + if (inbound->getLedger()->stateMap().family().isShardBacked()) + app_.getNodeStore().storeLedger(inbound->getLedger()); } - if (inbound->getLedger()->stateMap().family().isShardBacked()) - shardStore->setStored(inbound->getLedger()); - else - shardStore->storeLedger(inbound->getLedger()); - } - return inbound->getLedger(); + else if (reason == InboundLedger::Reason::SHARD) + { + auto shardStore = app_.getShardStore(); + if (!shardStore) + { + JLOG(j_.error()) + << "Acquiring shard with no shard store available"; + return {}; + } + if (inbound->getLedger()->stateMap().family().isShardBacked()) + shardStore->setStored(inbound->getLedger()); + else + shardStore->storeLedger(inbound->getLedger()); + } + return inbound->getLedger(); + }; + using namespace std::chrono_literals; + std::shared_ptr ledger = perf::measureDurationAndLog( + doAcquire, "InboundLedgersImp::acquire", 500ms, j_); + + return ledger; } std::shared_ptr diff --git a/src/ripple/app/main/Application.cpp b/src/ripple/app/main/Application.cpp index 55300a390c9..b1305e46672 100644 --- a/src/ripple/app/main/Application.cpp +++ b/src/ripple/app/main/Application.cpp @@ -942,7 +942,7 @@ class ApplicationImp : public Application, public BasicApp auto setup = setup_DatabaseCon(*config_, m_journal); setup.useGlobalPragma = false; - mWalletDB = makeWalletDB(setup); + mWalletDB = makeWalletDB(setup, m_journal); } catch (std::exception const& e) { diff --git a/src/ripple/app/main/Main.cpp b/src/ripple/app/main/Main.cpp index 710e4e9674f..02989b9ee6c 100644 --- a/src/ripple/app/main/Main.cpp +++ b/src/ripple/app/main/Main.cpp @@ -595,7 +595,7 @@ run(int argc, char** argv) try { auto setup = setup_DatabaseCon(*config); - if (!doVacuumDB(setup)) + if (!doVacuumDB(setup, config->journal())) return -1; } catch (std::exception const& e) diff --git a/src/ripple/app/rdb/Download.h b/src/ripple/app/rdb/Download.h index b72b5ec57e7..a16c9aa8788 100644 --- a/src/ripple/app/rdb/Download.h +++ b/src/ripple/app/rdb/Download.h @@ -36,13 +36,15 @@ namespace ripple { * download process or continues an existing one. * @param setup Path to the database and other opening parameters. * @param path Path of the new file to download. + * @param j Journal. * @return Pair containing a unique pointer to the database and the amount of * bytes already downloaded if a download is being continued. */ std::pair, std::optional> openDatabaseBodyDb( DatabaseCon::Setup const& setup, - boost::filesystem::path const& path); + boost::filesystem::path const& path, + beast::Journal j); /** * @brief databaseBodyDoPut Saves a new fragment of a downloaded file. diff --git a/src/ripple/app/rdb/ShardArchive.h b/src/ripple/app/rdb/ShardArchive.h index 20c4382b056..27db8279718 100644 --- a/src/ripple/app/rdb/ShardArchive.h +++ b/src/ripple/app/rdb/ShardArchive.h @@ -30,10 +30,14 @@ namespace ripple { * descriptor. * @param dir Path to the database to open. * @param dbName Name of the database. + * @param j Journal. * @return Unique pointer to the opened database. */ std::unique_ptr -makeArchiveDB(boost::filesystem::path const& dir, std::string const& dbName); +makeArchiveDB( + boost::filesystem::path const& dir, + std::string const& dbName, + beast::Journal j); /** * @brief readArchiveDB Reads entries from the shard archive database and diff --git a/src/ripple/app/rdb/UnitaryShard.h b/src/ripple/app/rdb/UnitaryShard.h index d2ac773dbd3..e9a2ac93d2d 100644 --- a/src/ripple/app/rdb/UnitaryShard.h +++ b/src/ripple/app/rdb/UnitaryShard.h @@ -39,13 +39,15 @@ struct DatabasePair * and returns their descriptors. * @param config Config object. * @param setup Path to the databases and other opening parameters. + * @param j Journal. * @return Pair of unique pointers to the opened ledger and transaction * databases. */ DatabasePair makeShardCompleteLedgerDBs( Config const& config, - DatabaseCon::Setup const& setup); + DatabaseCon::Setup const& setup, + beast::Journal j); /** * @brief makeShardIncompleteLedgerDBs Opens shard databases for partially @@ -53,6 +55,7 @@ makeShardCompleteLedgerDBs( * @param config Config object. * @param setup Path to the databases and other opening parameters. * @param checkpointerSetup Checkpointer parameters. + * @param j Journal. * @return Pair of unique pointers to the opened ledger and transaction * databases. */ @@ -60,7 +63,8 @@ DatabasePair makeShardIncompleteLedgerDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup); + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j); /** * @brief updateLedgerDBs Saves the given ledger to shard databases. @@ -86,12 +90,14 @@ updateLedgerDBs( * descriptor. * @param setup Path to the database and other opening parameters. * @param checkpointerSetup Checkpointer parameters. + * @param j Journal. * @return Unique pointer to the opened database. */ std::unique_ptr makeAcquireDB( DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup); + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j); /** * @brief insertAcquireDBIndex Adds a new shard index to the shard acquire diff --git a/src/ripple/app/rdb/Vacuum.h b/src/ripple/app/rdb/Vacuum.h index 3db18da045a..463f046e0a5 100644 --- a/src/ripple/app/rdb/Vacuum.h +++ b/src/ripple/app/rdb/Vacuum.h @@ -27,10 +27,11 @@ namespace ripple { /** * @brief doVacuumDB Creates, initialises, and performs cleanup on a database. * @param setup Path to the database and other opening parameters. + * @param j Journal. * @return True if the vacuum process completed successfully. */ bool -doVacuumDB(DatabaseCon::Setup const& setup); +doVacuumDB(DatabaseCon::Setup const& setup, beast::Journal j); } // namespace ripple diff --git a/src/ripple/app/rdb/Wallet.h b/src/ripple/app/rdb/Wallet.h index e9846714ece..6a15997ffe4 100644 --- a/src/ripple/app/rdb/Wallet.h +++ b/src/ripple/app/rdb/Wallet.h @@ -32,19 +32,24 @@ namespace ripple { /** * @brief makeWalletDB Opens the wallet database and returns it. * @param setup Path to the database and other opening parameters. + * @param j Journal. * @return Unique pointer to the database descriptor. */ std::unique_ptr -makeWalletDB(DatabaseCon::Setup const& setup); +makeWalletDB(DatabaseCon::Setup const& setup, beast::Journal j); /** * @brief makeTestWalletDB Opens a test wallet database with an arbitrary name. * @param setup Path to the database and other opening parameters. * @param dbname Name of the database. + * @param j Journal. * @return Unique pointer to the database descriptor. */ std::unique_ptr -makeTestWalletDB(DatabaseCon::Setup const& setup, std::string const& dbname); +makeTestWalletDB( + DatabaseCon::Setup const& setup, + std::string const& dbname, + beast::Journal j); /** * @brief getManifests Loads a manifest from the wallet database and stores it diff --git a/src/ripple/app/rdb/backend/detail/Node.h b/src/ripple/app/rdb/backend/detail/Node.h index ab3a99f5f95..2c3f264d7a9 100644 --- a/src/ripple/app/rdb/backend/detail/Node.h +++ b/src/ripple/app/rdb/backend/detail/Node.h @@ -47,6 +47,7 @@ struct DatabasePairValid * @param config Config object. * @param setup Path to database and opening parameters. * @param checkpointerSetup Database checkpointer setup. + * @param j Journal. * @return Struct DatabasePairValid which contain unique pointers to ledger * and transaction databases and flag if opening was successfull. */ @@ -54,7 +55,8 @@ DatabasePairValid makeLedgerDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup); + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j); /** * @brief getMinLedgerSeq Returns minimum ledger sequence in given table. diff --git a/src/ripple/app/rdb/backend/detail/Shard.h b/src/ripple/app/rdb/backend/detail/Shard.h index ac88c24bd78..02e75d2aa64 100644 --- a/src/ripple/app/rdb/backend/detail/Shard.h +++ b/src/ripple/app/rdb/backend/detail/Shard.h @@ -37,6 +37,7 @@ namespace detail { * @param config Config object. * @param setup Path to database and opening parameters. * @param checkpointerSetup Database checkpointer setup. + * @param j Journal. * @return Struct DatabasePair which contains unique pointers to the ledger * and transaction databases. */ @@ -44,7 +45,8 @@ DatabasePair makeMetaDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup); + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j); /** * @brief saveLedgerMeta Stores (transaction ID -> shard index) and diff --git a/src/ripple/app/rdb/backend/detail/impl/Node.cpp b/src/ripple/app/rdb/backend/detail/impl/Node.cpp index 0905d6121ae..894cae1385c 100644 --- a/src/ripple/app/rdb/backend/detail/impl/Node.cpp +++ b/src/ripple/app/rdb/backend/detail/impl/Node.cpp @@ -67,11 +67,12 @@ DatabasePairValid makeLedgerDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup) + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j) { // ledger database auto lgr{std::make_unique( - setup, LgrDBName, LgrDBPragma, LgrDBInit, checkpointerSetup)}; + setup, LgrDBName, LgrDBPragma, LgrDBInit, checkpointerSetup, j)}; lgr->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::lgrDBCache))); @@ -80,7 +81,7 @@ makeLedgerDBs( { // transaction database auto tx{std::make_unique( - setup, TxDBName, TxDBPragma, TxDBInit, checkpointerSetup)}; + setup, TxDBName, TxDBPragma, TxDBInit, checkpointerSetup, j)}; tx->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::txnDBCache))); diff --git a/src/ripple/app/rdb/backend/detail/impl/Shard.cpp b/src/ripple/app/rdb/backend/detail/impl/Shard.cpp index f7a0ce4571b..540e8d1d221 100644 --- a/src/ripple/app/rdb/backend/detail/impl/Shard.cpp +++ b/src/ripple/app/rdb/backend/detail/impl/Shard.cpp @@ -32,7 +32,8 @@ DatabasePair makeMetaDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup) + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j) { // ledger meta database auto lgrMetaDB{std::make_unique( @@ -40,14 +41,20 @@ makeMetaDBs( LgrMetaDBName, LgrMetaDBPragma, LgrMetaDBInit, - checkpointerSetup)}; + checkpointerSetup, + j)}; if (!config.useTxTables()) return {std::move(lgrMetaDB), nullptr}; // transaction meta database auto txMetaDB{std::make_unique( - setup, TxMetaDBName, TxMetaDBPragma, TxMetaDBInit, checkpointerSetup)}; + setup, + TxMetaDBName, + TxMetaDBPragma, + TxMetaDBInit, + checkpointerSetup, + j)}; return {std::move(lgrMetaDB), std::move(txMetaDB)}; } diff --git a/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp b/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp index 547ab843b36..05a460819d0 100644 --- a/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp +++ b/src/ripple/app/rdb/backend/impl/SQLiteDatabase.cpp @@ -447,7 +447,7 @@ SQLiteDatabaseImp::makeLedgerDBs( DatabaseCon::CheckpointerSetup const& checkpointerSetup) { auto [lgr, tx, res] = - detail::makeLedgerDBs(config, setup, checkpointerSetup); + detail::makeLedgerDBs(config, setup, checkpointerSetup, j_); txdb_ = std::move(tx); lgrdb_ = std::move(lgr); return res; @@ -460,7 +460,7 @@ SQLiteDatabaseImp::makeMetaDBs( DatabaseCon::CheckpointerSetup const& checkpointerSetup) { auto [lgrMetaDB, txMetaDB] = - detail::makeMetaDBs(config, setup, checkpointerSetup); + detail::makeMetaDBs(config, setup, checkpointerSetup, j_); txMetaDB_ = std::move(txMetaDB); lgrMetaDB_ = std::move(lgrMetaDB); diff --git a/src/ripple/app/rdb/impl/Download.cpp b/src/ripple/app/rdb/impl/Download.cpp index 0905ee577b1..6f6d6bf7577 100644 --- a/src/ripple/app/rdb/impl/Download.cpp +++ b/src/ripple/app/rdb/impl/Download.cpp @@ -25,14 +25,15 @@ namespace ripple { std::pair, std::optional> openDatabaseBodyDb( DatabaseCon::Setup const& setup, - boost::filesystem::path const& path) + boost::filesystem::path const& path, + beast::Journal j) { // SOCI requires boost::optional (not std::optional) as the parameter. boost::optional pathFromDb; boost::optional size; auto conn = std::make_unique( - setup, "Download", DownloaderDBPragma, DatabaseBodyDBInit); + setup, "Download", DownloaderDBPragma, DatabaseBodyDBInit, j); auto& session = *conn->checkoutDb(); diff --git a/src/ripple/app/rdb/impl/ShardArchive.cpp b/src/ripple/app/rdb/impl/ShardArchive.cpp index 6880aa00136..edad36f7099 100644 --- a/src/ripple/app/rdb/impl/ShardArchive.cpp +++ b/src/ripple/app/rdb/impl/ShardArchive.cpp @@ -22,10 +22,13 @@ namespace ripple { std::unique_ptr -makeArchiveDB(boost::filesystem::path const& dir, std::string const& dbName) +makeArchiveDB( + boost::filesystem::path const& dir, + std::string const& dbName, + beast::Journal j) { return std::make_unique( - dir, dbName, DownloaderDBPragma, ShardArchiveHandlerDBInit); + dir, dbName, DownloaderDBPragma, ShardArchiveHandlerDBInit, j); } void diff --git a/src/ripple/app/rdb/impl/UnitaryShard.cpp b/src/ripple/app/rdb/impl/UnitaryShard.cpp index ab1758b4852..786340f5955 100644 --- a/src/ripple/app/rdb/impl/UnitaryShard.cpp +++ b/src/ripple/app/rdb/impl/UnitaryShard.cpp @@ -27,16 +27,17 @@ namespace ripple { DatabasePair makeShardCompleteLedgerDBs( Config const& config, - DatabaseCon::Setup const& setup) + DatabaseCon::Setup const& setup, + beast::Journal j) { auto tx{std::make_unique( - setup, TxDBName, FinalShardDBPragma, TxDBInit)}; + setup, TxDBName, FinalShardDBPragma, TxDBInit, j)}; tx->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::txnDBCache, std::nullopt))); auto lgr{std::make_unique( - setup, LgrDBName, FinalShardDBPragma, LgrDBInit)}; + setup, LgrDBName, FinalShardDBPragma, LgrDBInit, j)}; lgr->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::lgrDBCache, std::nullopt))); @@ -48,18 +49,19 @@ DatabasePair makeShardIncompleteLedgerDBs( Config const& config, DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup) + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j) { // transaction database auto tx{std::make_unique( - setup, TxDBName, TxDBPragma, TxDBInit, checkpointerSetup)}; + setup, TxDBName, TxDBPragma, TxDBInit, checkpointerSetup, j)}; tx->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::txnDBCache))); // ledger database auto lgr{std::make_unique( - setup, LgrDBName, LgrDBPragma, LgrDBInit, checkpointerSetup)}; + setup, LgrDBName, LgrDBPragma, LgrDBInit, checkpointerSetup, j)}; lgr->getSession() << boost::str( boost::format("PRAGMA cache_size=-%d;") % kilobytes(config.getValueFor(SizedItem::lgrDBCache))); @@ -209,14 +211,16 @@ updateLedgerDBs( std::unique_ptr makeAcquireDB( DatabaseCon::Setup const& setup, - DatabaseCon::CheckpointerSetup const& checkpointerSetup) + DatabaseCon::CheckpointerSetup const& checkpointerSetup, + beast::Journal j) { return std::make_unique( setup, AcquireShardDBName, AcquireShardDBPragma, AcquireShardDBInit, - checkpointerSetup); + checkpointerSetup, + j); } void diff --git a/src/ripple/app/rdb/impl/Vacuum.cpp b/src/ripple/app/rdb/impl/Vacuum.cpp index aad456cc5a8..20386f07b18 100644 --- a/src/ripple/app/rdb/impl/Vacuum.cpp +++ b/src/ripple/app/rdb/impl/Vacuum.cpp @@ -23,7 +23,7 @@ namespace ripple { bool -doVacuumDB(DatabaseCon::Setup const& setup) +doVacuumDB(DatabaseCon::Setup const& setup, beast::Journal j) { boost::filesystem::path dbPath = setup.dataDir / TxDBName; @@ -41,7 +41,7 @@ doVacuumDB(DatabaseCon::Setup const& setup) } auto txnDB = - std::make_unique(setup, TxDBName, TxDBPragma, TxDBInit); + std::make_unique(setup, TxDBName, TxDBPragma, TxDBInit, j); auto& session = txnDB->getSession(); std::uint32_t pageSize; diff --git a/src/ripple/app/rdb/impl/Wallet.cpp b/src/ripple/app/rdb/impl/Wallet.cpp index 3715c4c7458..79734ab67dc 100644 --- a/src/ripple/app/rdb/impl/Wallet.cpp +++ b/src/ripple/app/rdb/impl/Wallet.cpp @@ -23,19 +23,22 @@ namespace ripple { std::unique_ptr -makeWalletDB(DatabaseCon::Setup const& setup) +makeWalletDB(DatabaseCon::Setup const& setup, beast::Journal j) { // wallet database return std::make_unique( - setup, WalletDBName, std::array(), WalletDBInit); + setup, WalletDBName, std::array(), WalletDBInit, j); } std::unique_ptr -makeTestWalletDB(DatabaseCon::Setup const& setup, std::string const& dbname) +makeTestWalletDB( + DatabaseCon::Setup const& setup, + std::string const& dbname, + beast::Journal j) { // wallet database return std::make_unique( - setup, dbname.data(), std::array(), WalletDBInit); + setup, dbname.data(), std::array(), WalletDBInit, j); } void diff --git a/src/ripple/basics/PerfLog.h b/src/ripple/basics/PerfLog.h index 3d1cb371715..49ed27338db 100644 --- a/src/ripple/basics/PerfLog.h +++ b/src/ripple/basics/PerfLog.h @@ -179,6 +179,30 @@ make_PerfLog( beast::Journal journal, std::function&& signalStop); +template +auto +measureDurationAndLog( + Func&& func, + const std::string& actionDescription, + std::chrono::duration maxDelay, + const beast::Journal& journal) +{ + auto start_time = std::chrono::high_resolution_clock::now(); + + auto result = func(); + + auto end_time = std::chrono::high_resolution_clock::now(); + auto duration = std::chrono::duration_cast( + end_time - start_time); + if (duration > maxDelay) + { + JLOG(journal.warn()) + << actionDescription << " took " << duration.count() << " ms"; + } + + return result; +} + } // namespace perf } // namespace ripple diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index cf41678a16c..25852615185 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -396,6 +396,12 @@ class Config : public BasicConfig int getValueFor(SizedItem item, std::optional node = std::nullopt) const; + + beast::Journal + journal() const + { + return j_; + } }; FeeSetup diff --git a/src/ripple/core/DatabaseCon.h b/src/ripple/core/DatabaseCon.h index 59eec3a32e9..91d2a446668 100644 --- a/src/ripple/core/DatabaseCon.h +++ b/src/ripple/core/DatabaseCon.h @@ -21,6 +21,7 @@ #define RIPPLE_APP_DATA_DATABASECON_H_INCLUDED #include +#include #include #include #include @@ -115,7 +116,8 @@ class DatabaseCon Setup const& setup, std::string const& dbName, std::array const& pragma, - std::array const& initSQL) + std::array const& initSQL, + beast::Journal journal) // Use temporary files or regular DB files? : DatabaseCon( setup.standAlone && !setup.reporting && @@ -126,7 +128,8 @@ class DatabaseCon : (setup.dataDir / dbName), setup.commonPragma(), pragma, - initSQL) + initSQL, + journal) { } @@ -137,8 +140,9 @@ class DatabaseCon std::string const& dbName, std::array const& pragma, std::array const& initSQL, - CheckpointerSetup const& checkpointerSetup) - : DatabaseCon(setup, dbName, pragma, initSQL) + CheckpointerSetup const& checkpointerSetup, + beast::Journal journal) + : DatabaseCon(setup, dbName, pragma, initSQL, journal) { setupCheckpointing(checkpointerSetup.jobQueue, *checkpointerSetup.logs); } @@ -148,8 +152,9 @@ class DatabaseCon boost::filesystem::path const& dataDir, std::string const& dbName, std::array const& pragma, - std::array const& initSQL) - : DatabaseCon(dataDir / dbName, nullptr, pragma, initSQL) + std::array const& initSQL, + beast::Journal journal) + : DatabaseCon(dataDir / dbName, nullptr, pragma, initSQL, journal) { } @@ -160,8 +165,9 @@ class DatabaseCon std::string const& dbName, std::array const& pragma, std::array const& initSQL, - CheckpointerSetup const& checkpointerSetup) - : DatabaseCon(dataDir, dbName, pragma, initSQL) + CheckpointerSetup const& checkpointerSetup, + beast::Journal journal) + : DatabaseCon(dataDir, dbName, pragma, initSQL, journal) { setupCheckpointing(checkpointerSetup.jobQueue, *checkpointerSetup.logs); } @@ -177,7 +183,14 @@ class DatabaseCon LockedSociSession checkoutDb() { - return LockedSociSession(session_, lock_); + using namespace std::chrono_literals; + LockedSociSession session = perf::measureDurationAndLog( + [&]() { return LockedSociSession(session_, lock_); }, + "checkoutDb", + 10ms, + j_); + + return session; } private: @@ -189,8 +202,9 @@ class DatabaseCon boost::filesystem::path const& pPath, std::vector const* commonPragma, std::array const& pragma, - std::array const& initSQL) - : session_(std::make_shared()) + std::array const& initSQL, + beast::Journal journal) + : session_(std::make_shared()), j_(journal) { open(*session_, "sqlite", pPath.string()); @@ -224,6 +238,8 @@ class DatabaseCon // shared_ptr in this class. session_ will never be null. std::shared_ptr const session_; std::shared_ptr checkpointer_; + + beast::Journal const j_; }; // Return the checkpointer from its id. If the checkpointer no longer exists, an diff --git a/src/ripple/net/DatabaseBody.h b/src/ripple/net/DatabaseBody.h index c828e5bf123..4c74b879a59 100644 --- a/src/ripple/net/DatabaseBody.h +++ b/src/ripple/net/DatabaseBody.h @@ -102,13 +102,16 @@ class DatabaseBody::value_type @param io_service The asio context for running a strand. @param ec Set to the error, if any occurred + + @param j Journal. */ void open( boost::filesystem::path const& path, Config const& config, boost::asio::io_service& io_service, - boost::system::error_code& ec); + boost::system::error_code& ec, + beast::Journal j); }; /** Algorithm for storing buffers when parsing. diff --git a/src/ripple/net/DatabaseDownloader.h b/src/ripple/net/DatabaseDownloader.h index 3e920909417..476939e1ff4 100644 --- a/src/ripple/net/DatabaseDownloader.h +++ b/src/ripple/net/DatabaseDownloader.h @@ -43,7 +43,8 @@ class DatabaseDownloader : public HTTPDownloader getParser( boost::filesystem::path dstPath, std::function complete, - boost::system::error_code& ec) override; + boost::system::error_code& ec, + beast::Journal j) override; bool checkPath(boost::filesystem::path const& dstPath) override; diff --git a/src/ripple/net/HTTPDownloader.h b/src/ripple/net/HTTPDownloader.h index 39b9a904aa3..34e81ea1f06 100644 --- a/src/ripple/net/HTTPDownloader.h +++ b/src/ripple/net/HTTPDownloader.h @@ -113,7 +113,8 @@ class HTTPDownloader : public std::enable_shared_from_this getParser( boost::filesystem::path dstPath, std::function complete, - boost::system::error_code& ec) = 0; + boost::system::error_code& ec, + beast::Journal j) = 0; virtual bool checkPath(boost::filesystem::path const& dstPath) = 0; diff --git a/src/ripple/net/impl/DatabaseBody.ipp b/src/ripple/net/impl/DatabaseBody.ipp index cdc7da2bc41..875a883527f 100644 --- a/src/ripple/net/impl/DatabaseBody.ipp +++ b/src/ripple/net/impl/DatabaseBody.ipp @@ -46,7 +46,8 @@ DatabaseBody::value_type::open( boost::filesystem::path const& path, Config const& config, boost::asio::io_service& io_service, - boost::system::error_code& ec) + boost::system::error_code& ec, + beast::Journal j) { strand_.reset(new boost::asio::io_service::strand(io_service)); path_ = path; @@ -55,7 +56,7 @@ DatabaseBody::value_type::open( setup.dataDir = path.parent_path(); setup.useGlobalPragma = false; - auto [conn, size] = openDatabaseBodyDb(setup, path); + auto [conn, size] = openDatabaseBodyDb(setup, path, j); conn_ = std::move(conn); if (size) fileSize_ = *size; diff --git a/src/ripple/net/impl/DatabaseDownloader.cpp b/src/ripple/net/impl/DatabaseDownloader.cpp index eab0d74d7d9..52388cf0d1d 100644 --- a/src/ripple/net/impl/DatabaseDownloader.cpp +++ b/src/ripple/net/impl/DatabaseDownloader.cpp @@ -45,13 +45,14 @@ auto DatabaseDownloader::getParser( boost::filesystem::path dstPath, std::function complete, - boost::system::error_code& ec) -> std::shared_ptr + boost::system::error_code& ec, + beast::Journal j) -> std::shared_ptr { using namespace boost::beast; auto p = std::make_shared>(); p->body_limit(std::numeric_limits::max()); - p->get().body().open(dstPath, config_, io_service_, ec); + p->get().body().open(dstPath, config_, io_service_, ec, j); if (ec) p->get().body().close(); diff --git a/src/ripple/net/impl/HTTPDownloader.cpp b/src/ripple/net/impl/HTTPDownloader.cpp index 44d27466224..43bf8d92589 100644 --- a/src/ripple/net/impl/HTTPDownloader.cpp +++ b/src/ripple/net/impl/HTTPDownloader.cpp @@ -146,7 +146,7 @@ HTTPDownloader::do_session( if (stop_.load()) return exit(); - auto p = this->getParser(dstPath, complete, ec); + auto p = this->getParser(dstPath, complete, ec, j_); if (ec) return failAndExit("getParser", p); diff --git a/src/ripple/nodestore/impl/Shard.cpp b/src/ripple/nodestore/impl/Shard.cpp index 10adf298361..f05f56903d2 100644 --- a/src/ripple/nodestore/impl/Shard.cpp +++ b/src/ripple/nodestore/impl/Shard.cpp @@ -862,7 +862,8 @@ Shard::open(std::lock_guard const& lock) acquireInfo_ = std::make_unique(); acquireInfo_->SQLiteDB = makeAcquireDB( setup, - DatabaseCon::CheckpointerSetup{&app_.getJobQueue(), &app_.logs()}); + DatabaseCon::CheckpointerSetup{&app_.getJobQueue(), &app_.logs()}, + j_); state_ = ShardState::acquire; progress_ = 0; @@ -981,7 +982,7 @@ Shard::initSQLite(std::lock_guard const&) case ShardState::complete: case ShardState::finalizing: case ShardState::finalized: { - auto [lgr, tx] = makeShardCompleteLedgerDBs(config, setup); + auto [lgr, tx] = makeShardCompleteLedgerDBs(config, setup, j_); lgrSQLiteDB_ = std::move(lgr); lgrSQLiteDB_->getSession() << boost::str( @@ -1005,7 +1006,8 @@ Shard::initSQLite(std::lock_guard const&) config, setup, DatabaseCon::CheckpointerSetup{ - &app_.getJobQueue(), &app_.logs()}); + &app_.getJobQueue(), &app_.logs()}, + j_); lgrSQLiteDB_ = std::move(lgr); lgrSQLiteDB_->getSession() << boost::str( diff --git a/src/ripple/overlay/impl/PeerImp.cpp b/src/ripple/overlay/impl/PeerImp.cpp index f93c9f135ad..69ef6061d59 100644 --- a/src/ripple/overlay/impl/PeerImp.cpp +++ b/src/ripple/overlay/impl/PeerImp.cpp @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -920,8 +921,16 @@ PeerImp::onReadMessage(error_code ec, std::size_t bytes_transferred) while (read_buffer_.size() > 0) { std::size_t bytes_consumed; - std::tie(bytes_consumed, ec) = - invokeProtocolMessage(read_buffer_.data(), *this, hint); + + using namespace std::chrono_literals; + std::tie(bytes_consumed, ec) = perf::measureDurationAndLog( + [&]() { + return invokeProtocolMessage(read_buffer_.data(), *this, hint); + }, + "invokeProtocolMessage", + 350ms, + journal_); + if (ec) return fail("onReadMessage", ec); if (!socket_.is_open()) diff --git a/src/ripple/rpc/impl/ShardArchiveHandler.cpp b/src/ripple/rpc/impl/ShardArchiveHandler.cpp index d05744f483a..90bd5edc361 100644 --- a/src/ripple/rpc/impl/ShardArchiveHandler.cpp +++ b/src/ripple/rpc/impl/ShardArchiveHandler.cpp @@ -114,7 +114,7 @@ ShardArchiveHandler::init() { create_directories(downloadDir_); - sqlDB_ = makeArchiveDB(downloadDir_, stateDBName); + sqlDB_ = makeArchiveDB(downloadDir_, stateDBName, j_); } catch (std::exception const& e) { @@ -139,7 +139,7 @@ ShardArchiveHandler::initFromDB(std::lock_guard const& lock) exists(downloadDir_ / stateDBName) && is_regular_file(downloadDir_ / stateDBName)); - sqlDB_ = makeArchiveDB(downloadDir_, stateDBName); + sqlDB_ = makeArchiveDB(downloadDir_, stateDBName, j_); readArchiveDB(*sqlDB_, [&](std::string const& url_, int state) { parsedURL url; diff --git a/src/test/app/Manifest_test.cpp b/src/test/app/Manifest_test.cpp index b72623309e9..0f21cab0d63 100644 --- a/src/test/app/Manifest_test.cpp +++ b/src/test/app/Manifest_test.cpp @@ -255,7 +255,7 @@ class Manifest_test : public beast::unit_test::suite setup.dataDir = getDatabasePath(); assert(!setup.useGlobalPragma); - auto dbCon = makeTestWalletDB(setup, dbName); + auto dbCon = makeTestWalletDB(setup, dbName, env.journal); auto getPopulatedManifests = [](ManifestCache const& cache) -> std::vector { From 774148389467781aca7c01bac90af2fba870570c Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Sun, 25 Aug 2024 19:10:07 -0700 Subject: [PATCH 2/6] Allow only 1 job queue slot for acquiring inbound ledger. * Log when duplicate concurrent inbound ledger are filtered. * RAII for containers that track concurrent inbound ledger. * Comment on when to asynchronously acquire inbound ledgers, which is possible to be always OK, but should have further review. * Other small logging changes Co-authored-by: Ed Hennis --- src/ripple/app/consensus/RCLConsensus.cpp | 8 +++-- src/ripple/app/consensus/RCLValidations.cpp | 6 ++-- src/ripple/app/ledger/InboundLedgers.h | 14 ++++++-- src/ripple/app/ledger/impl/InboundLedger.cpp | 2 +- src/ripple/app/ledger/impl/InboundLedgers.cpp | 35 +++++++++++++++++++ src/ripple/app/misc/NetworkOPs.cpp | 3 +- src/test/app/LedgerReplay_test.cpp | 8 +++++ 7 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/ripple/app/consensus/RCLConsensus.cpp b/src/ripple/app/consensus/RCLConsensus.cpp index c0ebe06dd7e..0335e9979d2 100644 --- a/src/ripple/app/consensus/RCLConsensus.cpp +++ b/src/ripple/app/consensus/RCLConsensus.cpp @@ -135,8 +135,12 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash) acquiringLedger_ = hash; app_.getJobQueue().addJob( - jtADVANCE, "getConsensusLedger", [id = hash, &app = app_]() { - app.getInboundLedgers().acquire( + jtADVANCE, + "getConsensusLedger1", + [id = hash, &app = app_, this]() { + JLOG(j_.debug()) + << "JOB advanceLedger getConsensusLedger1 started"; + app.getInboundLedgers().acquireAsync( id, 0, InboundLedger::Reason::CONSENSUS); }); } diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index 6b626569e69..9ffd8829ccc 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -142,8 +142,10 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash) Application* pApp = &app_; app_.getJobQueue().addJob( - jtADVANCE, "getConsensusLedger", [pApp, hash]() { - pApp->getInboundLedgers().acquire( + jtADVANCE, "getConsensusLedger2", [pApp, hash, this]() { + JLOG(j_.debug()) + << "JOB advanceLedger getConsensusLedger2 started"; + pApp->getInboundLedgers().acquireAsync( hash, 0, InboundLedger::Reason::CONSENSUS); }); return std::nullopt; diff --git a/src/ripple/app/ledger/InboundLedgers.h b/src/ripple/app/ledger/InboundLedgers.h index b12760153e2..aa173925893 100644 --- a/src/ripple/app/ledger/InboundLedgers.h +++ b/src/ripple/app/ledger/InboundLedgers.h @@ -23,6 +23,7 @@ #include #include #include +#include namespace ripple { @@ -37,11 +38,20 @@ class InboundLedgers virtual ~InboundLedgers() = default; - // VFALCO TODO Should this be called findOrAdd ? - // + // Callers should use this if they possibly need an authoritative + // response immediately. virtual std::shared_ptr acquire(uint256 const& hash, std::uint32_t seq, InboundLedger::Reason) = 0; + // Callers should use this if they are known to be executing on the Job + // Queue. TODO review whether all callers of acquire() can use this + // instead. Inbound ledger acquisition is asynchronous anyway. + virtual void + acquireAsync( + uint256 const& hash, + std::uint32_t seq, + InboundLedger::Reason reason) = 0; + virtual std::shared_ptr find(LedgerHash const& hash) = 0; diff --git a/src/ripple/app/ledger/impl/InboundLedger.cpp b/src/ripple/app/ledger/impl/InboundLedger.cpp index 53475988cbf..c8dc005097b 100644 --- a/src/ripple/app/ledger/impl/InboundLedger.cpp +++ b/src/ripple/app/ledger/impl/InboundLedger.cpp @@ -536,7 +536,7 @@ InboundLedger::trigger(std::shared_ptr const& peer, TriggerReason reason) return; } - if (auto stream = journal_.trace()) + if (auto stream = journal_.debug()) { stream << "Trigger acquiring ledger " << hash_; if (peer) diff --git a/src/ripple/app/ledger/impl/InboundLedgers.cpp b/src/ripple/app/ledger/impl/InboundLedgers.cpp index b9b8b9fcfd2..f88137e8501 100644 --- a/src/ripple/app/ledger/impl/InboundLedgers.cpp +++ b/src/ripple/app/ledger/impl/InboundLedgers.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -149,6 +150,37 @@ class InboundLedgersImp : public InboundLedgers return ledger; } + void + acquireAsync( + uint256 const& hash, + std::uint32_t seq, + InboundLedger::Reason reason) override + { + std::unique_lock lock(acquiresMutex_); + try + { + if (pendingAcquires_.contains(hash)) + return; + pendingAcquires_.insert(hash); + lock.unlock(); + acquire(hash, seq, reason); + } + catch (std::exception const& e) + { + JLOG(j_.warn()) + << "Exception thrown for acquiring new inbound ledger " << hash + << ": " << e.what(); + } + catch (...) + { + JLOG(j_.warn()) + << "Unknown exception thrown for acquiring new inbound ledger " + << hash; + } + lock.lock(); + pendingAcquires_.erase(hash); + } + std::shared_ptr find(uint256 const& hash) override { @@ -441,6 +473,9 @@ class InboundLedgersImp : public InboundLedgers beast::insight::Counter mCounter; std::unique_ptr mPeerSetBuilder; + + std::set pendingAcquires_; + std::mutex acquiresMutex_; }; //------------------------------------------------------------------------------ diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index cd85bc9e4e1..5f72ce9cfa0 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -1730,7 +1730,8 @@ NetworkOPsImp::checkLastClosedLedger( } JLOG(m_journal.warn()) << "We are not running on the consensus ledger"; - JLOG(m_journal.info()) << "Our LCL: " << getJson({*ourClosed, {}}); + JLOG(m_journal.info()) << "Our LCL: " << ourClosed->info().hash + << getJson({*ourClosed, {}}); JLOG(m_journal.info()) << "Net LCL " << closedLedger; if ((mMode == OperatingMode::TRACKING) || (mMode == OperatingMode::FULL)) diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp index aee24cd7d57..b312fb69fe0 100644 --- a/src/test/app/LedgerReplay_test.cpp +++ b/src/test/app/LedgerReplay_test.cpp @@ -106,6 +106,14 @@ class MagicInboundLedgers : public InboundLedgers return {}; } + virtual void + acquireAsync( + uint256 const& hash, + std::uint32_t seq, + InboundLedger::Reason reason) override + { + } + virtual std::shared_ptr find(LedgerHash const& hash) override { From fbbea9e6e25795a8a6bd1bf64b780771933a9579 Mon Sep 17 00:00:00 2001 From: Mark Travis Date: Sun, 25 Aug 2024 19:20:14 -0700 Subject: [PATCH 3/6] Allow only 1 job queue slot for each validation ledger check * refactor filtering of validations to specifically avoid concurrent checkAccept() calls for the same validation ledger hash. * Log when duplicate concurrent validation requests are filtered. * RAII for containers that track concurrent validation requests. --- src/ripple/app/consensus/RCLValidations.cpp | 20 ++++++++++-- src/ripple/app/consensus/RCLValidations.h | 8 ++++- src/ripple/app/misc/NetworkOPs.cpp | 35 ++++++++++++++++++++- 3 files changed, 59 insertions(+), 4 deletions(-) diff --git a/src/ripple/app/consensus/RCLValidations.cpp b/src/ripple/app/consensus/RCLValidations.cpp index 9ffd8829ccc..b5069bd25c9 100644 --- a/src/ripple/app/consensus/RCLValidations.cpp +++ b/src/ripple/app/consensus/RCLValidations.cpp @@ -161,7 +161,9 @@ void handleNewValidation( Application& app, std::shared_ptr const& val, - std::string const& source) + std::string const& source, + BypassAccept const bypassAccept, + std::optional j) { auto const& signingKey = val->getSignerPublic(); auto const& hash = val->getLedgerHash(); @@ -186,7 +188,21 @@ handleNewValidation( if (outcome == ValStatus::current) { if (val->isTrusted()) - app.getLedgerMaster().checkAccept(hash, seq); + { + if (bypassAccept == BypassAccept::yes) + { + assert(j.has_value()); + if (j.has_value()) + { + JLOG(j->trace()) << "Bypassing checkAccept for validation " + << val->getLedgerHash(); + } + } + else + { + app.getLedgerMaster().checkAccept(hash, seq); + } + } return; } diff --git a/src/ripple/app/consensus/RCLValidations.h b/src/ripple/app/consensus/RCLValidations.h index 93628fe1695..e141731e14d 100644 --- a/src/ripple/app/consensus/RCLValidations.h +++ b/src/ripple/app/consensus/RCLValidations.h @@ -25,12 +25,16 @@ #include #include #include +#include +#include #include namespace ripple { class Application; +enum class BypassAccept : bool { no = false, yes }; + /** Wrapper over STValidation for generic Validation code Wraps an STValidation for compatibility with the generic validation code. @@ -248,7 +252,9 @@ void handleNewValidation( Application& app, std::shared_ptr const& val, - std::string const& source); + std::string const& source, + BypassAccept const bypassAccept = BypassAccept::no, + std::optional j = std::nullopt); } // namespace ripple diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index 5f72ce9cfa0..e2f5ef1a0b3 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -72,8 +72,10 @@ #include #include +#include #include #include +#include #include #include #include @@ -781,6 +783,9 @@ class NetworkOPsImp final : public NetworkOPs StateAccounting accounting_{}; + std::set pendingValidations_; + std::mutex validationsMutex_; + private: struct Stats { @@ -2308,7 +2313,35 @@ NetworkOPsImp::recvValidation( JLOG(m_journal.trace()) << "recvValidation " << val->getLedgerHash() << " from " << source; - handleNewValidation(app_, val, source); + std::unique_lock lock(validationsMutex_); + BypassAccept bypassAccept = BypassAccept::no; + try + { + if (pendingValidations_.contains(val->getLedgerHash())) + bypassAccept = BypassAccept::yes; + else + pendingValidations_.insert(val->getLedgerHash()); + lock.unlock(); + handleNewValidation(app_, val, source, bypassAccept, m_journal); + } + catch (std::exception const& e) + { + JLOG(m_journal.warn()) + << "Exception thrown for handling new validation " + << val->getLedgerHash() << ": " << e.what(); + } + catch (...) + { + JLOG(m_journal.warn()) + << "Unknown exception thrown for handling new validation " + << val->getLedgerHash(); + } + if (bypassAccept == BypassAccept::no) + { + lock.lock(); + pendingValidations_.erase(val->getLedgerHash()); + lock.unlock(); + } pubValidation(val); From 85214bdf811e1ff11a02977211fe59b07619b856 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Fri, 23 Aug 2024 17:19:52 -0400 Subject: [PATCH 4/6] Set version to 2.2.2 --- src/ripple/protocol/impl/BuildInfo.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ripple/protocol/impl/BuildInfo.cpp b/src/ripple/protocol/impl/BuildInfo.cpp index e6359cd3a52..0086824b7e2 100644 --- a/src/ripple/protocol/impl/BuildInfo.cpp +++ b/src/ripple/protocol/impl/BuildInfo.cpp @@ -33,7 +33,7 @@ namespace BuildInfo { // and follow the format described at http://semver.org/ //------------------------------------------------------------------------------ // clang-format off -char const* const versionString = "2.2.1" +char const* const versionString = "2.2.2" // clang-format on #if defined(DEBUG) || defined(SANITIZER) From cc0177be8712ca367134aa6599b7328bbc5e6bc5 Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Tue, 3 Sep 2024 18:37:47 -0400 Subject: [PATCH 5/6] Update Release Notes for 2.2.1 and 2.2.2 --- RELEASENOTES.md | 109 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/RELEASENOTES.md b/RELEASENOTES.md index cfc0e618cfe..72eb4687232 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -6,6 +6,115 @@ This document contains the release notes for `rippled`, the reference server imp Have new ideas? Need help with setting up your node? [Please open an issue here](https://github.com/xrplf/rippled/issues/new/choose). +# Version 2.2.2 + +Version 2.2.2 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release fixes an ongoing issue with Mainnet where validators can stall during consensus processing due to lock contention, preventing ledgers from being validated for up to two minutes. There are no new amendments in this release. + +[Sign Up for Future Release Announcements](https://groups.google.com/g/ripple-server) + + + +## Action Required + +If you run an XRP Ledger validator, upgrade to version 2.2.2 as soon as possible to ensure stable and uninterrupted network behavior. + +Additionally, five amendments introduced in version 2.2.0 are open for voting according to the XRP Ledger's [amendment process](https://xrpl.org/amendments.html), which enables protocol changes following two weeks of >80% support from trusted validators. If you operate an XRP Ledger server older than version 2.2.0, upgrade by September 17, 2024 to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. Version 2.2.2 is recommended because of known bugs affecting stability of versions 2.2.0 and 2.2.1. + +If you operate a Clio server, Clio needs to be updated to 2.1.2 before updating to rippled 2.2.0. Clio will be blocked if it is not updated. + +## Changelog + +### Amendments and New Features + +- None + +### Bug Fixes and Performance Improvements + +- Allow only 1 job queue slot for acquiring inbound ledger [#5115](https://github.com/XRPLF/rippled/pull/5115) ([7741483](https://github.com/XRPLF/rippled/commit/774148389467781aca7c01bac90af2fba870570c)) + +- Allow only 1 job queue slot for each validation ledger check [#5115](https://github.com/XRPLF/rippled/pull/5115) ([fbbea9e](https://github.com/XRPLF/rippled/commit/fbbea9e6e25795a8a6bd1bf64b780771933a9579)) + +### Other improvements + + - Track latencies of certain code blocks, and log if they take too long [#5115](https://github.com/XRPLF/rippled/pull/5115) ([00ed7c9](https://github.com/XRPLF/rippled/commit/00ed7c942436f02644a13169002b5123f4e2a116)) + +### Docs and Build System + +- None + +### GitHub + +The public source code repository for `rippled` is hosted on GitHub at . + +We welcome all contributions and invite everyone to join the community of XRP Ledger developers to help build the Internet of Value. + + +## Credits + +The following people contributed directly to this release: + +Mark Travis +Valentin Balaschenko <13349202+vlntb@users.noreply.github.com> + +Bug Bounties and Responsible Disclosures: + +We welcome reviews of the `rippled` code and urge researchers to responsibly disclose any issues they may find. + +To report a bug, please send a detailed report to: + +# Version 2.2.1 + +Version 2.2.1 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release fixes a critical bug introduced in 2.2.0 handling some types of RPC requests. + +[Sign Up for Future Release Announcements](https://groups.google.com/g/ripple-server) + + + +## Action Required + +If you run an XRP Ledger validator, upgrade to version 2.2.1 as soon as possible to ensure stable and uninterrupted network behavior. + +Additionally, five amendments introduced in version 2.2.0 are open for voting according to the XRP Ledger's [amendment process](https://xrpl.org/amendments.html), which enables protocol changes following two weeks of >80% support from trusted validators. If you operate an XRP Ledger server older than version 2.2.0, upgrade by August 14, 2024 to ensure service continuity. The exact time that protocol changes take effect depends on the voting decisions of the decentralized network. Version 2.2.1 is recommended because of known bugs affecting stability of versions 2.2.0. + +If you operate a Clio server, Clio needs to be updated to 2.2.2 before updating to rippled 2.2.1. Clio will be blocked if it is not updated. + +## Changelog + +### Amendments and New Features + +- None + +### Bug Fixes and Performance Improvements + +- Improve error handling in some RPC commands. [#5078](https://github.com/XRPLF/rippled/pull/5078) + +- Use error codes throughout fast Base58 implementation. [#5078](https://github.com/XRPLF/rippled/pull/5078) + +### Docs and Build System + +- None + +### GitHub + +The public source code repository for `rippled` is hosted on GitHub at . + +We welcome all contributions and invite everyone to join the community of XRP Ledger developers to help build the Internet of Value. + + +## Credits + +The following people contributed directly to this release: + +John Freeman +Mayukha Vadari + +Bug Bounties and Responsible Disclosures: + +We welcome reviews of the `rippled` code and urge researchers to responsibly disclose any issues they may find. + +To report a bug, please send a detailed report to: + + # Version 2.2.0 Version 2.2.0 of `rippled`, the reference server implementation of the XRP Ledger protocol, is now available. This release adds performance optimizations, several bug fixes, and introduces the `featurePriceOracle`, `fixEmptyDID`, `fixXChainRewardRounding`, `fixPreviousTxnID`, and `fixAMMv1_1` amendments. From 23991c99c31ed93ee875a077c7b8312a3ee7f53d Mon Sep 17 00:00:00 2001 From: Ed Hennis Date: Wed, 11 Sep 2024 11:29:06 +0100 Subject: [PATCH 6/6] test: Retry RPC commands to try to fix MacOS CI jobs (#5120) * Retry some failed RPC connections / commands in unit tests * Remove orphaned `getAccounts` function Co-authored-by: John Freeman --- src/test/jtx/impl/Env.cpp | 28 ++++++++++++++++++---------- src/xrpld/app/misc/NetworkOPs.cpp | 18 ------------------ 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/src/test/jtx/impl/Env.cpp b/src/test/jtx/impl/Env.cpp index 6f0f9e3fc73..ef5a2124e24 100644 --- a/src/test/jtx/impl/Env.cpp +++ b/src/test/jtx/impl/Env.cpp @@ -317,16 +317,24 @@ Env::submit(JTx const& jt) auto const jr = [&]() { if (jt.stx) { - txid_ = jt.stx->getTransactionID(); - Serializer s; - jt.stx->add(s); - auto const jr = rpc("submit", strHex(s.slice())); - - parsedResult = parseResult(jr); - test.expect(parsedResult.ter, "ter uninitialized!"); - ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED); - - return jr; + // We shouldn't need to retry, but it fixes the test on macOS for + // the moment. + int retries = 3; + do + { + txid_ = jt.stx->getTransactionID(); + Serializer s; + jt.stx->add(s); + auto const jr = rpc("submit", strHex(s.slice())); + + parsedResult = parseResult(jr); + test.expect(parsedResult.ter, "ter uninitialized!"); + ter_ = parsedResult.ter.value_or(telENV_RPC_FAILED); + if (ter_ != telENV_RPC_FAILED || + parsedResult.rpcCode != rpcINTERNAL || + jt.ter == telENV_RPC_FAILED || --retries <= 0) + return jr; + } while (true); } else { diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index 01906d306cd..02eb0435b57 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -2783,24 +2783,6 @@ NetworkOPsImp::pubProposedTransaction( pubProposedAccountTransaction(ledger, transaction, result); } -static void -getAccounts(Json::Value const& jvObj, std::vector& accounts) -{ - for (auto& jv : jvObj) - { - if (jv.isObject()) - { - getAccounts(jv, accounts); - } - else if (jv.isString()) - { - auto account = RPC::accountFromStringStrict(jv.asString()); - if (account) - accounts.push_back(*account); - } - } -} - void NetworkOPsImp::pubLedger(std::shared_ptr const& lpAccepted) {