From 2529edd2b6d107256170ddcc1045f69a29a0d954 Mon Sep 17 00:00:00 2001 From: Nik Bougalis Date: Wed, 13 Feb 2019 20:11:11 -0800 Subject: [PATCH] Properly transition state to disconnected: If the number of peers a server has is below the configured minimum peer limit, this commit will properly transition the server's state to "disconnected". The default limit for the minimum number of peers required was 0 meaning that a server that was connected but lost all its peers would never transition to disconnected, since it could never drop below zero peers. This commit redefines the default minimum number of peers to 1 and produces a warning if the server is configured in a way that will prevent it from ever achieving sufficient connectivity. --- src/ripple/app/misc/NetworkOPs.cpp | 23 +++++++++++------------ src/ripple/app/misc/NetworkOPs.h | 2 +- src/ripple/core/Config.h | 5 +++-- src/ripple/core/impl/Config.cpp | 22 ++++++++++++++++++++-- 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/ripple/app/misc/NetworkOPs.cpp b/src/ripple/app/misc/NetworkOPs.cpp index c38746654..b8bec3776 100644 --- a/src/ripple/app/misc/NetworkOPs.cpp +++ b/src/ripple/app/misc/NetworkOPs.cpp @@ -196,7 +196,7 @@ public: // VFALCO TODO Make LedgerMaster a SharedPtr or a reference. // NetworkOPsImp (Application& app, NetworkOPs::clock_type& clock, - bool standalone, std::size_t network_quorum, bool start_valid, + bool standalone, std::size_t minPeerCount, bool start_valid, JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, beast::Journal journal) @@ -220,7 +220,7 @@ public: , m_ledgerMaster (ledgerMaster) , m_job_queue (job_queue) , m_standalone (standalone) - , m_network_quorum (start_valid ? 0 : network_quorum) + , minPeerCount_ (start_valid ? 0 : minPeerCount) { } @@ -598,7 +598,7 @@ private: bool const m_standalone; // The number of nodes that we need to consider ourselves connected. - std::size_t const m_network_quorum; + std::size_t const minPeerCount_; // Transaction batching. std::condition_variable mCond; @@ -738,19 +738,19 @@ void NetworkOPsImp::processHeartbeatTimer () std::size_t const numPeers = app_.overlay ().size (); // do we have sufficient peers? If not, we are disconnected. - if (numPeers < m_network_quorum) + if (numPeers < minPeerCount_) { if (mMode != omDISCONNECTED) { setMode (omDISCONNECTED); JLOG(m_journal.warn()) - << "Node count (" << numPeers << ") " - << "has fallen below quorum (" << m_network_quorum << ")."; + << "Node count (" << numPeers << ") has fallen " + << "below required minimum (" << minPeerCount_ << ")."; } - // We do not call mConsensus.timerEntry until there - // are enough peers providing meaningful inputs to consensus - setHeartbeatTimer (); + // We do not call mConsensus.timerEntry until there are enough + // peers providing meaningful inputs to consensus + setHeartbeatTimer (); return; } @@ -767,7 +767,6 @@ void NetworkOPsImp::processHeartbeatTimer () setMode (omSYNCING); else if (mMode == omCONNECTED) setMode (omCONNECTED); - } mConsensus.timerEntry (app_.timeKeeper().closeTime()); @@ -3383,13 +3382,13 @@ NetworkOPsImp::StateAccounting::json() const std::unique_ptr make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, - bool standalone, std::size_t network_quorum, bool startvalid, + bool standalone, std::size_t minPeerCount, bool startvalid, JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, beast::Journal journal) { return std::make_unique (app, clock, standalone, - network_quorum, startvalid, job_queue, ledgerMaster, parent, + minPeerCount, startvalid, job_queue, ledgerMaster, parent, validatorKeys, io_svc, journal); } diff --git a/src/ripple/app/misc/NetworkOPs.h b/src/ripple/app/misc/NetworkOPs.h index 6a1648507..45704e9d3 100644 --- a/src/ripple/app/misc/NetworkOPs.h +++ b/src/ripple/app/misc/NetworkOPs.h @@ -238,7 +238,7 @@ public: std::unique_ptr make_NetworkOPs (Application& app, NetworkOPs::clock_type& clock, - bool standalone, std::size_t network_quorum, bool start_valid, + bool standalone, std::size_t minPeerCount, bool start_valid, JobQueue& job_queue, LedgerMaster& ledgerMaster, Stoppable& parent, ValidatorKeys const & validatorKeys, boost::asio::io_service& io_svc, beast::Journal journal); diff --git a/src/ripple/core/Config.h b/src/ripple/core/Config.h index 6f32c88d2..daf71dbcf 100644 --- a/src/ripple/core/Config.h +++ b/src/ripple/core/Config.h @@ -141,11 +141,12 @@ public: int const TRANSACTION_FEE_BASE = 10; // The number of fee units a reference transaction costs // Note: The following parameters do not relate to the UNL or trust at all - std::size_t NETWORK_QUORUM = 0; // Minimum number of nodes to consider the network present + // Minimum number of nodes to consider the network present + std::size_t NETWORK_QUORUM = 1; // Peer networking parameters bool PEER_PRIVATE = false; // True to ask peers not to relay current IP. - int PEERS_MAX = 0; + std::size_t PEERS_MAX = 0; std::chrono::seconds WEBSOCKET_PING_FREQ = std::chrono::minutes {5}; diff --git a/src/ripple/core/impl/Config.cpp b/src/ripple/core/impl/Config.cpp index cbe60af9f..37b9d157e 100644 --- a/src/ripple/core/impl/Config.cpp +++ b/src/ripple/core/impl/Config.cpp @@ -314,7 +314,7 @@ void Config::loadFromString (std::string const& fileContents) PEER_PRIVATE = beast::lexicalCastThrow (strTemp); if (getSingleSection (secConfig, SECTION_PEERS_MAX, strTemp, j_)) - PEERS_MAX = std::max (0, beast::lexicalCastThrow (strTemp)); + PEERS_MAX = beast::lexicalCastThrow (strTemp); if (getSingleSection (secConfig, SECTION_NODE_SIZE, strTemp, j_)) { @@ -360,7 +360,7 @@ void Config::loadFromString (std::string const& fileContents) "and [" SECTION_VALIDATOR_TOKEN "] config sections"); if (getSingleSection (secConfig, SECTION_NETWORK_QUORUM, strTemp, j_)) - NETWORK_QUORUM = beast::lexicalCastThrow (strTemp); + NETWORK_QUORUM = beast::lexicalCastThrow(strTemp); if (getSingleSection (secConfig, SECTION_FEE_ACCOUNT_RESERVE, strTemp, j_)) FEE_ACCOUNT_RESERVE = beast::lexicalCastThrow (strTemp); @@ -539,6 +539,24 @@ void Config::loadFromString (std::string const& fileContents) "Unknown feature: " + s + " in config file."); } } + + // This doesn't properly belong here, but check to make sure that the + // value specified for network_quorum is achievable: + { + auto pm = PEERS_MAX; + + // FIXME this apparently magic value is actually defined as a constant + // elsewhere (see defaultMaxPeers) but we handle this check here. + if (pm == 0) + pm = 21; + + if (NETWORK_QUORUM > pm) + { + Throw( + "The minimum number of required peers (network_quorum) exceeds " + "the maximum number of allowed peers (peers_max)"); + } + } } int Config::getSize (SizedItemName item) const