InboundLedgers improvements:

* Change findCreate to acquire
* Return Ledger rather than InboundLedger

src/ripple/app/consensus/LedgerConsensus.cpp

@@ -548,7 +548,7 @@ public:
                 mAcquiringLedger = mPrevLedgerHash;
                 getApp().getJobQueue().addJob (jtADVANCE, "getConsensusLedger",
                     std::bind (
-                        &InboundLedgers::findCreate,
+                        &InboundLedgers::acquire,
                         &getApp().getInboundLedgers(),
                         mPrevLedgerHash, 0, InboundLedger::fcCONSENSUS));
                 mHaveCorrectLCL = false;

src/ripple/app/ledger/InboundLedgers.cpp

@@ -47,12 +47,10 @@ public:
     {
     }
 
-    // VFALCO TODO Should this be called findOrAdd ?
+    Ledger::pointer acquire (uint256 const& hash, std::uint32_t seq, InboundLedger::fcReason reason)
-    //
-    InboundLedger::pointer findCreate (uint256 const& hash, std::uint32_t seq, InboundLedger::fcReason reason)
     {
         assert (hash.isNonZero ());
-        InboundLedger::pointer ret;
+        Ledger::pointer ret;
 
         {
             ScopedLockType sl (mLock);
@@ -62,15 +60,16 @@ public:
                 auto it = mLedgers.find (hash);
                 if (it != mLedgers.end ())
                 {
-                    ret = it->second;
+                    it->second->update (seq);
-                    ret->update (seq);
+                    if (it->second->isComplete() && !it->second->isFailed())
+                        ret = it->second->getLedger();
+
                 }
                 else
                 {
-                    ret = std::make_shared <InboundLedger> (hash, seq, reason, std::ref (m_clock));
+                    auto il = std::make_shared <InboundLedger> (hash, seq, reason, std::ref (m_clock));
-                    assert (ret);
+                    mLedgers.insert (std::make_pair (hash, il));
-                    mLedgers.insert (std::make_pair (hash, ret));
+                    il->init (sl);
-                    ret->init (sl);
                     ++mCounter;
                 }
             }

src/ripple/app/ledger/InboundLedgers.h

@@ -40,7 +40,7 @@ public:
 
     // VFALCO TODO Should this be called findOrAdd ?
     //
-    virtual InboundLedger::pointer findCreate (uint256 const& hash,
+    virtual Ledger::pointer acquire (uint256 const& hash,
         std::uint32_t seq, InboundLedger::fcReason) = 0;
 
     virtual InboundLedger::pointer find (LedgerHash const& hash) = 0;

src/ripple/app/ledger/Ledger.cpp

@@ -1274,7 +1274,7 @@ void Ledger::visitStateItems (std::function<void (SLE::ref)> function) const
     {
         if (mHash.isNonZero ())
         {
-            getApp().getInboundLedgers().findCreate(
+            getApp().getInboundLedgers().acquire(
                 mHash, mLedgerSeq, InboundLedger::fcGENERIC);
         }
         throw;

src/ripple/app/ledger/LedgerCleaner.cpp

@@ -259,7 +259,7 @@ public:
         {
             m_journal.warning <<
                 "Node missing from ledger " << ledger->getLedgerSeq();
-            getApp().getInboundLedgers().findCreate (
+            getApp().getInboundLedgers().acquire (
                 ledger->getHash(), ledger->getLedgerSeq(), InboundLedger::fcGENERIC);
         }
         return hash;
@@ -278,7 +278,9 @@ public:
         bool doNodes,
         bool doTxns)
     {
-        Ledger::pointer nodeLedger = getApp().getLedgerMaster().findAcquireLedger(ledgerIndex, ledgerHash);
+        Ledger::pointer nodeLedger =
+            getApp().getInboundLedgers().acquire (
+                ledgerHash, ledgerIndex, InboundLedger::fcGENERIC);
         if (!nodeLedger)
         {
             m_journal.debug << "Ledger " << ledgerIndex << " not available";
@@ -305,7 +307,8 @@ public:
         if (doNodes && !nodeLedger->walkLedger())
         {
             m_journal.debug << "Ledger " << ledgerIndex << " is missing nodes";
-            getApp().getInboundLedgers().findCreate(ledgerHash, ledgerIndex, InboundLedger::fcGENERIC);
+            getApp().getInboundLedgers().acquire(
+                ledgerHash, ledgerIndex, InboundLedger::fcGENERIC);
             return false;
         }
 
@@ -355,7 +358,9 @@ public:
                 if (nonzero)
                 {
                     // We found the hash and sequence of a better reference ledger
-                    referenceLedger = getApp().getLedgerMaster().findAcquireLedger (refIndex, refHash);
+                    referenceLedger =
+                        getApp().getInboundLedgers().acquire(
+                            refHash, refIndex, InboundLedger::fcGENERIC);
                     if (referenceLedger)
                         ledgerHash = getLedgerHash(referenceLedger, ledgerIndex);
                 }

src/ripple/app/ledger/LedgerMaster.cpp

@@ -705,7 +705,7 @@ public:
     void failedSave(std::uint32_t seq, uint256 const& hash)
     {
         clearLedger(seq);
-        getApp().getInboundLedgers().findCreate(hash, seq, InboundLedger::fcGENERIC);
+        getApp().getInboundLedgers().acquire(hash, seq, InboundLedger::fcGENERIC);
     }
 
     // Check if the specified ledger can become the new last fully-validated ledger
@@ -739,15 +739,8 @@ public:
 
             // FIXME: We may not want to fetch a ledger with just one
             // trusted validation
-            InboundLedger::pointer l =
+            ledger =
-                getApp().getInboundLedgers().findCreate(hash, 0, InboundLedger::fcGENERIC);
+                getApp().getInboundLedgers().acquire(hash, 0, InboundLedger::fcGENERIC);
-            if (l && l->isComplete() && !l->isFailed())
-                ledger = l->getLedger();
-            else
-            {
-                WriteLog (lsDEBUG, LedgerMaster) <<
-                    "checkAccept triggers acquire " << to_string (hash);
-            }
         }
 
         if (ledger)
@@ -989,21 +982,11 @@ public:
                                 {
                                     if (!getApp().getInboundLedgers().isFailure(nextLedger->getParentHash()))
                                     {
-                                        InboundLedger::pointer acq =
+                                        ledger =
-                                            getApp().getInboundLedgers().findCreate(nextLedger->getParentHash(),
+                                            getApp().getInboundLedgers().acquire(nextLedger->getParentHash(),
-                                                                                    nextLedger->getLedgerSeq() - 1,
+                                                                                 nextLedger->getLedgerSeq() - 1,
-                                                                                    InboundLedger::fcHISTORY);
+                                                                                 InboundLedger::fcHISTORY);
-                                        if (!acq)
+                                        if (! ledger && (missing > 32600) && getApp().getOPs().shouldFetchPack(missing))
-                                        {
-                                            // On system shutdown, findCreate may return a nullptr
-                                            WriteLog (lsTRACE, LedgerMaster)
-                                                    << "findCreate failed to return an inbound ledger";
-                                            return;
-                                        }
-
-                                        if (acq->isComplete() && !acq->isFailed())
-                                            ledger = acq->getLedger();
-                                        else if ((missing > 40000) && getApp().getOPs().shouldFetchPack(missing))
                                         {
                                             WriteLog (lsTRACE, LedgerMaster) << "tryAdvance want fetch pack " << missing;
                                             getFetchPack(nextLedger);
@@ -1038,7 +1021,7 @@ public:
                                             std::uint32_t seq = missing - i;
                                             uint256 hash = nextLedger->getLedgerHash(seq);
                                             if (hash.isNonZero())
-                                                getApp().getInboundLedgers().findCreate(hash,
+                                                getApp().getInboundLedgers().acquire(hash,
                                                      seq, InboundLedger::fcHISTORY);
                                         }
                                     }
@@ -1151,47 +1134,10 @@ public:
                         ledger = mLedgerHistory.getLedgerByHash (hash);
                     }
 
-                    if (!ledger && (++acqCount < 4))
+                    if (! ledger && (++acqCount < 4))
                     { // We can try to acquire the ledger we need
-                        InboundLedger::pointer acq =
+                        ledger =
-                            getApp().getInboundLedgers ().findCreate (hash, seq, InboundLedger::fcGENERIC);
+                            getApp().getInboundLedgers ().acquire (hash, seq, InboundLedger::fcGENERIC);
-
-                        if (!acq)
-                        {
-                            // On system shutdown, findCreate may return a nullptr
-                            WriteLog (lsTRACE, LedgerMaster)
-                                << "findCreate failed to return an inbound ledger";
-                            return {};
-                        }
-
-                        if (!acq->isDone()) {
-                        }
-                        else if (acq->isComplete () && !acq->isFailed ())
-                        {
-                            ledger = acq->getLedger();
-                        }
-                        else
-                        {
-                            WriteLog (lsWARNING, LedgerMaster) << "Failed to acquire a published ledger";
-                            getApp().getInboundLedgers().dropLedger(hash);
-                            acq = getApp().getInboundLedgers().findCreate(hash, seq, InboundLedger::fcGENERIC);
-
-                            if (!acq)
-                            {
-                                // On system shutdown, findCreate may return a nullptr
-                                WriteLog (lsTRACE, LedgerMaster)
-                                    << "findCreate failed to return an inbound ledger";
-                                return {};
-                            }
-
-                            if (acq->isComplete())
-                            {
-                                if (acq->isFailed())
-                                    getApp().getInboundLedgers().dropLedger(hash);
-                                else
-                                    ledger = acq->getLedger();
-                            }
-                        }
                     }
 
                     if (ledger && (ledger->getLedgerSeq() == pubSeq))
@@ -1311,7 +1257,7 @@ public:
             catch (SHAMapMissingNode&)
             {
                 WriteLog (lsINFO, LedgerMaster) << "Missing node detected during pathfinding";
-                getApp().getInboundLedgers().findCreate(lastLedger->getHash (), lastLedger->getLedgerSeq (),
+                getApp().getInboundLedgers().acquire(lastLedger->getHash (), lastLedger->getLedgerSeq (),
                     InboundLedger::fcGENERIC);
             }
         }
@@ -1402,22 +1348,6 @@ public:
         return mCompleteLedgers.toString ();
     }
 
-    /** Find or acquire the ledger with the specified index and the specified hash
-        Return a pointer to that ledger if it is immediately available
-    */
-    Ledger::pointer findAcquireLedger (std::uint32_t index, uint256 const& hash)
-    {
-        Ledger::pointer ledger (getLedgerByHash (hash));
-        if (!ledger)
-        {
-            InboundLedger::pointer inboundLedger =
-                getApp().getInboundLedgers().findCreate (hash, index, InboundLedger::fcGENERIC);
-            if (inboundLedger && inboundLedger->isComplete() && !inboundLedger->isFailed())
-                ledger = inboundLedger->getLedger();
-        }
-        return ledger;
-    }
-
     uint256 getHashBySeq (std::uint32_t index)
     {
         uint256 hash = mLedgerHistory.getLedgerHash (index);
@@ -1466,7 +1396,9 @@ public:
             if (nonzero)
             {
                 // We found the hash and sequence of a better reference ledger
-                Ledger::pointer ledger = findAcquireLedger (refIndex, refHash);
+                Ledger::pointer ledger =
+                    getApp().getInboundLedgers().acquire (
+                        refHash, refIndex, InboundLedger::fcGENERIC);
                 if (ledger)
                 {
                     ledgerHash = ledger->getLedgerHash (index);

src/ripple/app/ledger/LedgerMaster.h

@@ -109,8 +109,6 @@ public:
     virtual uint256 walkHashBySeq (std::uint32_t index) = 0;
     virtual uint256 walkHashBySeq (std::uint32_t index, Ledger::ref referenceLedger) = 0;
 
-    virtual Ledger::pointer findAcquireLedger (std::uint32_t index, uint256 const& hash) = 0;
-
     virtual Ledger::pointer getLedgerBySeq (std::uint32_t index) = 0;
 
     virtual Ledger::pointer getLedgerByHash (uint256 const& hash) = 0;

src/ripple/app/misc/NetworkOPs.cpp

@@ -1431,17 +1431,7 @@ bool NetworkOPsImp::checkLastClosedLedger (
         networkClosed = closedLedger;
 
     if (!switchLedgers)
-    {
-        if (mAcquiringLedger)
-        {
-            mAcquiringLedger->abort ();
-            getApp().getInboundLedgers ().dropLedger (
-                mAcquiringLedger->getHash ());
-            mAcquiringLedger.reset ();
-        }
-
         return false;
-    }
 
     m_journal.warning << "We are not running on the consensus ledger";
     m_journal.info << "Our LCL: " << getJson (*ourClosed);
@@ -1453,31 +1443,18 @@ bool NetworkOPsImp::checkLastClosedLedger (
     Ledger::pointer consensus = m_ledgerMaster.getLedgerByHash (closedLedger);
 
     if (!consensus)
+        consensus = getApp().getInboundLedgers().acquire (
+            closedLedger, 0, InboundLedger::fcCONSENSUS);
+
+    if (consensus)
     {
-        m_journal.info << "Acquiring consensus ledger " << closedLedger;
-
-        if (!mAcquiringLedger || (mAcquiringLedger->getHash () != closedLedger))
-            mAcquiringLedger = getApp().getInboundLedgers ().findCreate (
-                closedLedger, 0, InboundLedger::fcCONSENSUS);
-
-        if (!mAcquiringLedger || mAcquiringLedger->isFailed ())
-        {
-            getApp().getInboundLedgers ().dropLedger (closedLedger);
-            m_journal.error << "Network ledger cannot be acquired";
-            return true;
-        }
-
-        if (!mAcquiringLedger->isComplete ())
-            return true;
-
         clearNeedNetworkLedger ();
-        consensus = mAcquiringLedger->getLedger ();
-    }
 
-    // FIXME: If this rewinds the ledger sequence, or has the same sequence, we
+        // FIXME: If this rewinds the ledger sequence, or has the same sequence, we
-    // should update the status on any stored transactions in the invalidated
+        // should update the status on any stored transactions in the invalidated
-    // ledgers.
+        // ledgers.
-    switchLastClosedLedger (consensus, false);
+        switchLastClosedLedger (consensus, false);
+    }
 
     return true;
 }
@@ -3359,7 +3336,7 @@ void NetworkOPsImp::missingNodeInLedger (std::uint32_t seq)
     else
     {
         m_journal.warning << "Missing a node in ledger " << seq << " fetching";
-        getApp().getInboundLedgers ().findCreate (
+        getApp().getInboundLedgers ().acquire (
             hash, seq, InboundLedger::fcGENERIC);
     }
 }

src/ripple/rpc/handlers/LedgerRequest.cpp

@@ -77,8 +77,11 @@ Json::Value doLedgerRequest (RPC::Context& context)
                 // We don't have the ledger we need to figure out which ledger
                 // they want. Try to get it.
 
-                if (auto il = getApp().getInboundLedgers().findCreate (
+                if (auto il = getApp().getInboundLedgers().acquire (
                         refHash, refIndex, InboundLedger::fcGENERIC))
+                    return getJson (LedgerFill (*il));
+
+                if (auto il = getApp().getInboundLedgers().find (refHash))
                 {
                     Json::Value jvResult = il->getJson (0);
 
@@ -86,7 +89,7 @@ Json::Value doLedgerRequest (RPC::Context& context)
                     return jvResult;
                 }
 
-                // findCreate failed to return an inbound ledger. App is likely shutting down
+                // Likely the app is shutting down
                 return Json::Value();
             }
 
@@ -107,11 +110,13 @@ Json::Value doLedgerRequest (RPC::Context& context)
     else
     {
         // Try to get the desired ledger
-        if (auto il = getApp ().getInboundLedgers ().findCreate (
+        if (auto il = getApp ().getInboundLedgers ().acquire (
                 ledgerHash, 0, InboundLedger::fcGENERIC))
-        {
+            return getJson (LedgerFill (*il));
+
+        if (auto il = getApp().getInboundLedgers().find (ledgerHash))
             return il->getJson (0);
-        }
+
         return RPC::make_error (
             rpcNOT_READY, "findCreate failed to return an inbound ledger");
     }