diff --git a/src/ripple_app/misc/NetworkOPs.cpp b/src/ripple_app/misc/NetworkOPs.cpp index 17062d9ae6..0f21a979bd 100644 --- a/src/ripple_app/misc/NetworkOPs.cpp +++ b/src/ripple_app/misc/NetworkOPs.cpp @@ -243,7 +243,7 @@ public: void mapComplete (uint256 const& hash, SHAMap::ref map); bool stillNeedTXSet (uint256 const& hash); void makeFetchPack (Job&, boost::weak_ptr peer, boost::shared_ptr request, - Ledger::pointer wantLedger, Ledger::pointer haveLedger, std::uint32_t uUptime); + uint256 haveLedger, std::uint32_t uUptime); bool shouldFetchPack (std::uint32_t seq); void gotFetchPack (bool progress, std::uint32_t seq); void addFetchPack (uint256 const& hash, boost::shared_ptr< Blob >& data); @@ -3191,8 +3191,7 @@ static void fpAppender (protocol::TMGetObjectByHash* reply, std::uint32_t ledger void NetworkOPsImp::makeFetchPack (Job&, boost::weak_ptr wPeer, boost::shared_ptr request, - Ledger::pointer wantLedger, Ledger::pointer haveLedger, - std::uint32_t uUptime) + uint256 haveLedgerHash, std::uint32_t uUptime) { if (UptimeTimer::getInstance ().getElapsedSeconds () > (uUptime + 1)) { @@ -3200,18 +3199,54 @@ void NetworkOPsImp::makeFetchPack (Job&, boost::weak_ptr wPeer, return; } - if (getApp().getFeeTrack ().isLoadedLocal ()) + if (getApp().getFeeTrack ().isLoadedLocal () || + (m_ledgerMaster.getValidatedLedgerAge() > 40)) { m_journal.info << "Too busy to make fetch pack"; return; } + Peer::ptr peer = wPeer.lock (); + + if (!peer) + return; + + Ledger::pointer haveLedger = getLedgerByHash (haveLedgerHash); + + if (!haveLedger) + { + m_journal.info << "Peer requests fetch pack for ledger we don't have: " << haveLedger; + peer->charge (Resource::feeRequestNoReply); + return; + } + + if (!haveLedger->isClosed ()) + { + m_journal.warning << "Peer requests fetch pack from open ledger: " << haveLedger; + peer->charge (Resource::feeInvalidRequest); + return; + } + + if (haveLedger->getLedgerSeq() < m_ledgerMaster.getEarliestFetch()) + { + m_journal.debug << "Peer requests fetch pack that is too early"; + peer->charge (Resource::feeInvalidRequest); + return; + } + + Ledger::pointer wantLedger = getLedgerByHash (haveLedger->getParentHash ()); + + if (!wantLedger) + { + m_journal.info + << "Peer requests fetch pack for ledger whose predecessor we don't have: " + << haveLedger; + peer->charge (Resource::feeRequestNoReply); + return; + } + try { - Peer::ptr peer = wPeer.lock (); - - if (!peer) - return; protocol::TMGetObjectByHash reply; reply.set_query (false); @@ -3234,7 +3269,8 @@ void NetworkOPsImp::makeFetchPack (Job&, boost::weak_ptr wPeer, newObj.set_data (s.getDataPtr (), s.getLength ()); newObj.set_ledgerseq (lSeq); - wantLedger->peekAccountStateMap ()->getFetchPack (haveLedger->peekAccountStateMap ().get (), true, 1024, + wantLedger->peekAccountStateMap ()->getFetchPack + (haveLedger->peekAccountStateMap ().get (), true, 1024, BIND_TYPE (fpAppender, &reply, lSeq, P_1, P_2)); if (wantLedger->getTransHash ().isNonZero ()) @@ -3244,7 +3280,7 @@ void NetworkOPsImp::makeFetchPack (Job&, boost::weak_ptr wPeer, if (reply.objects ().size () >= 256) break; - // VFALCO NOTE Why use move? + // move may save a ref/unref haveLedger = std::move (wantLedger); wantLedger = getLedgerByHash (haveLedger->getParentHash ()); } diff --git a/src/ripple_app/misc/NetworkOPs.h b/src/ripple_app/misc/NetworkOPs.h index 2777b14c99..402a5f71a3 100644 --- a/src/ripple_app/misc/NetworkOPs.h +++ b/src/ripple_app/misc/NetworkOPs.h @@ -228,7 +228,7 @@ public: // Fetch packs virtual void makeFetchPack (Job&, boost::weak_ptr peer, boost::shared_ptr request, - Ledger::pointer wantLedger, Ledger::pointer haveLedger, std::uint32_t uUptime) = 0; + uint256 wantLedger, std::uint32_t uUptime) = 0; virtual bool shouldFetchPack (std::uint32_t seq) = 0; virtual void gotFetchPack (bool progress, std::uint32_t seq) = 0; diff --git a/src/ripple_app/shamap/SHAMapSync.cpp b/src/ripple_app/shamap/SHAMapSync.cpp index fba8c407bc..66f573d245 100644 --- a/src/ripple_app/shamap/SHAMapSync.cpp +++ b/src/ripple_app/shamap/SHAMapSync.cpp @@ -635,16 +635,13 @@ std::list SHAMap::getFetchPack (SHAMap* have, bool inc void SHAMap::getFetchPack (SHAMap* have, bool includeLeaves, int max, std::function func) { - ScopedReadLockType ul1 (mLock); - - std::unique_ptr ul2; + ScopedReadLockType ul1 (mLock), ul2; if (have) { - // VFALCO NOTE This looks like a mess. A dynamically allocated scoped lock? - ul2.reset (new ScopedReadLockType (have->mLock, boost::try_to_lock)); + ul2 = std::move (ScopedReadLockType (have->mLock, boost::try_to_lock)); - if (! ul2->owns_lock ()) + if (! ul2.owns_lock ()) { WriteLog (lsINFO, SHAMap) << "Unable to create pack due to lock"; return; diff --git a/src/ripple_overlay/impl/PeerImp.h b/src/ripple_overlay/impl/PeerImp.h index 8c701090f8..e15199ef24 100644 --- a/src/ripple_overlay/impl/PeerImp.h +++ b/src/ripple_overlay/impl/PeerImp.h @@ -2556,7 +2556,11 @@ private: void doFetchPack (const boost::shared_ptr& packet) { // VFALCO TODO Invert this dependency using an observer and shared state object. - if (getApp().getFeeTrack ().isLoadedLocal ()) + // Don't queue fetch pack jobs if we're under load or we already have + // some queued. + if (getApp().getFeeTrack ().isLoadedLocal () || + (getApp().getLedgerMaster().getValidatedLedgerAge() > 40) || + (getApp().getJobQueue().getJobCount(jtPACK) > 10)) { m_journal.info << "Too busy to make fetch pack"; return; @@ -2572,41 +2576,10 @@ private: uint256 hash; memcpy (hash.begin (), packet->ledgerhash ().data (), 32); - Ledger::pointer haveLedger = getApp().getOPs ().getLedgerByHash (hash); - - if (!haveLedger) - { - m_journal.info << "Peer requests fetch pack for ledger we don't have: " << hash; - charge (Resource::feeRequestNoReply); - return; - } - - if (!haveLedger->isClosed ()) - { - m_journal.warning << "Peer requests fetch pack from open ledger: " << hash; - charge (Resource::feeInvalidRequest); - return; - } - - if (haveLedger->getLedgerSeq() < getApp().getLedgerMaster().getEarliestFetch()) - { - m_journal.debug << "Peer requests fetch pack that is too early"; - charge (Resource::feeInvalidRequest); - return; - } - - Ledger::pointer wantLedger = getApp().getOPs ().getLedgerByHash (haveLedger->getParentHash ()); - - if (!wantLedger) - { - m_journal.info << "Peer requests fetch pack for ledger whose predecessor we don't have: " << hash; - charge (Resource::feeRequestNoReply); - return; - } - getApp().getJobQueue ().addJob (jtPACK, "MakeFetchPack", - BIND_TYPE (&NetworkOPs::makeFetchPack, &getApp().getOPs (), P_1, - boost::weak_ptr (shared_from_this ()), packet, wantLedger, haveLedger, UptimeTimer::getInstance ().getElapsedSeconds ())); + BIND_TYPE (&NetworkOPs::makeFetchPack, &getApp().getOPs (), P_1, + boost::weak_ptr (shared_from_this ()), packet, + hash, UptimeTimer::getInstance ().getElapsedSeconds ())); } void doProofOfWork (Job&, boost::weak_ptr peer, ProofOfWork::pointer pow)