Revert "More logging checkAccept"

This reverts commit fb34346. Revert "More logging of checkAccept" This reverts commit a8f2dea. Revert "Time checkAccept for each validation." This reverts commit 91216d8. Revert "Log each check accept from validation hash and seq to check for dups." This reverts commit 2bbdbb7. Revert "Only have 1 running job for each validation message." This reverts commit dac66ee. Revert "Only have 1 running job for each inbound ledger." This reverts commit ba45f49.
mtrippled · Aug 26, 2024 · f8e57cb · f8e57cb
1 parent fb34346
commit f8e57cb
Show file tree

Hide file tree

Showing 9 changed files with 13 additions and 89 deletions.
diff --git a/src/test/app/LedgerReplay_test.cpp b/src/test/app/LedgerReplay_test.cpp
@@ -92,8 +92,7 @@ class MagicInboundLedgers : public InboundLedgers
     virtual ~MagicInboundLedgers() = default;
 
     virtual std::shared_ptr<Ledger const>
-    acquire(uint256 const& hash, std::uint32_t seq, InboundLedger::Reason,
-        bool jq)
+    acquire(uint256 const& hash, std::uint32_t seq, InboundLedger::Reason)
         override
     {
         if (bhvr == InboundLedgersBehavior::DropAll)

diff --git a/src/xrpld/app/consensus/RCLConsensus.cpp b/src/xrpld/app/consensus/RCLConsensus.cpp
@@ -137,7 +137,7 @@ RCLConsensus::Adaptor::acquireLedger(LedgerHash const& hash)
                 jtADVANCE1, "getConsensusLedger1", [id = hash, &app = app_, this]() {
                     JLOG(j_.debug()) << "JOB advanceLedger getConsensusLedger1 started";
                     app.getInboundLedgers().acquire(
-                        id, 0, InboundLedger::Reason::CONSENSUS, true);
+                        id, 0, InboundLedger::Reason::CONSENSUS);
                     JLOG(j_.debug()) << "JOB advanceLedger getConsensusLedger1 finishing";
                 });
         }
@@ -881,7 +881,7 @@ RCLConsensus::Adaptor::validate(
     // suppress it if we receive it
     app_.getHashRouter().addSuppression(sha512Half(makeSlice(serialized)));
 
-    handleNewValidation(app_, v, "local", j_);
+    handleNewValidation(app_, v, "local");
 
     // Broadcast to all our peers:
     protocol::TMValidation val;

diff --git a/src/xrpld/app/consensus/RCLValidations.cpp b/src/xrpld/app/consensus/RCLValidations.cpp
@@ -138,7 +138,7 @@ RCLValidationsAdaptor::acquire(LedgerHash const& hash)
             jtADVANCE2, "getConsensusLedger2", [pApp, hash, this]() {
                 JLOG(j_.debug()) << "JOB advanceLedger getConsensusLedger2 started";
                 pApp->getInboundLedgers().acquire(
-                    hash, 0, InboundLedger::Reason::CONSENSUS, true);
+                    hash, 0, InboundLedger::Reason::CONSENSUS);
                 JLOG(j_.debug()) << "JOB advanceLedger getConsensusLedger2 finishing";
             });
         return std::nullopt;
@@ -154,9 +154,7 @@ void
 handleNewValidation(
     Application& app,
     std::shared_ptr<STValidation> const& val,
-    std::string const& source,
-    beast::Journal j,
-    bool jq)
+    std::string const& source)
 {
     auto const& signingKey = val->getSignerPublic();
     auto const& hash = val->getLedgerHash();
@@ -180,11 +178,8 @@ handleNewValidation(
 
     if (outcome == ValStatus::current)
     {
-        auto const start = std::chrono::steady_clock::now();
         if (val->isTrusted())
-            app.getLedgerMaster().checkAccept(hash, seq, jq);
-        JLOG(j.debug()) << "checkAccept validation hash seq durationus " << hash
-            << ' ' << seq << ' ' << std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::steady_clock::now() - start).count() << "us";
+            app.getLedgerMaster().checkAccept(hash, seq);
         return;
     }
 

diff --git a/src/xrpld/app/consensus/RCLValidations.h b/src/xrpld/app/consensus/RCLValidations.h
@@ -243,15 +243,12 @@ using RCLValidations = Validations<RCLValidationsAdaptor>;
     @param app Application object containing validations and ledgerMaster
     @param val The validation to add
     @param source Name associated with validation used in logging
-    @param jq Whether this is being run on the Job Queue.
 */
 void
 handleNewValidation(
     Application& app,
     std::shared_ptr<STValidation> const& val,
-    std::string const& source,
-    beast::Journal j,
-    bool jq = false);
+    std::string const& source);
 
 }  // namespace ripple
 

diff --git a/src/xrpld/app/ledger/InboundLedgers.h b/src/xrpld/app/ledger/InboundLedgers.h
@@ -40,8 +40,7 @@ class InboundLedgers
     // VFALCO TODO Should this be called findOrAdd ?
     //
     virtual std::shared_ptr<Ledger const>
-    acquire(uint256 const& hash, std::uint32_t seq, InboundLedger::Reason,
-        bool jq = false) = 0;
+    acquire(uint256 const& hash, std::uint32_t seq, InboundLedger::Reason) = 0;
 
     virtual std::shared_ptr<InboundLedger>
     find(LedgerHash const& hash) = 0;

diff --git a/src/xrpld/app/ledger/LedgerMaster.h b/src/xrpld/app/ledger/LedgerMaster.h
@@ -229,7 +229,7 @@ class LedgerMaster : public AbstractFetchPackContainer
     void
     checkAccept(std::shared_ptr<Ledger const> const& ledger);
     void
-    checkAccept(uint256 const& hash, std::uint32_t seq, bool jq = false);
+    checkAccept(uint256 const& hash, std::uint32_t seq);
     void
     consensusBuilt(
         std::shared_ptr<Ledger const> const& ledger,

diff --git a/src/xrpld/app/ledger/detail/InboundLedgers.cpp b/src/xrpld/app/ledger/detail/InboundLedgers.cpp
@@ -30,7 +30,6 @@
 #include <memory>
 #include <mutex>
 #include <thread>
-#include <set>
 #include <vector>
 
 namespace ripple {
@@ -68,8 +67,7 @@ class InboundLedgersImp : public InboundLedgers
     acquire(
         uint256 const& hash,
         std::uint32_t seq,
-        InboundLedger::Reason reason,
-        bool jq) override
+        InboundLedger::Reason reason) override
     {
         static std::size_t instance = 0;
         ++instance;
@@ -133,21 +131,9 @@ class InboundLedgersImp : public InboundLedgers
 
         if (!isNew)
         {
-            if (jq)
-            {
-                std::lock_guard _(mLock);
-                if (pendingJobs_.contains(*const_cast<uint256*>(&hash)))
-                    return {};
-                pendingJobs_.insert(*const_cast<uint256*>(&hash));
-            }
             JLOG(j_.debug()) << "InboundLedgers::acquire5.4 " << this << " " << std::this_thread::get_id() << " " << instance;
             inbound->update(seq);
             JLOG(j_.debug()) << "InboundLedgers::acquire5.5 " << this << " " << std::this_thread::get_id() << " " << instance;
-            if (jq)
-            {
-                std::lock_guard _(mLock);
-                pendingJobs_.erase(*const_cast<uint256 *>(&hash));
-            }
         }
 
         if (!inbound->isComplete())
@@ -493,8 +479,6 @@ class InboundLedgersImp : public InboundLedgers
     beast::insight::Counter mCounter;
 
     std::unique_ptr<PeerSetBuilder> mPeerSetBuilder;
-
-    std::set<uint256> pendingJobs_;
 };
 
 //------------------------------------------------------------------------------

diff --git a/src/xrpld/app/ledger/detail/LedgerMaster.cpp b/src/xrpld/app/ledger/detail/LedgerMaster.cpp
@@ -1000,26 +1000,15 @@ LedgerMaster::failedSave(std::uint32_t seq, uint256 const& hash)
 // Check if the specified ledger can become the new last fully-validated
 // ledger.
 void
-LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq, bool jq)
+LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq)
 {
-    if (jq)
-    {
-        JLOG(m_journal.debug()) << "checkAccept jq validation1 " << hash << " " << seq;
-    }
-
     std::size_t valCount = 0;
 
     if (seq != 0)
     {
         // Ledger is too old
         if (seq < mValidLedgerSeq)
-        {
-            if (jq)
-            {
-                JLOG(m_journal.debug()) << "checkAccept jq validation1.1 " << hash << " " << seq;
-            }
             return;
-        }
 
         auto validations = app_.validators().negativeUNLFilter(
             app_.getValidations().getTrustedForLedger(hash, seq));
@@ -1032,34 +1021,14 @@ LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq, bool jq)
         }
 
         if (seq == mValidLedgerSeq)
-        {
-            if (jq)
-            {
-                JLOG(m_journal.debug()) << "checkAccept jq validation1.2 " << hash << " " << seq;
-            }
             return;
-        }
 
         // Ledger could match the ledger we're already building
         if (seq == mBuildingLedgerSeq)
-        {
-            if (jq)
-            {
-                JLOG(m_journal.debug()) << "checkAccept jq validation1.3 " << hash << " " << seq;
-            }
             return;
-        }
     }
 
-    if (jq)
-    {
-        JLOG(m_journal.debug()) << "checkAccept jq validation2 " << hash << " " << seq;
-    }
     auto ledger = mLedgerHistory.getLedgerByHash(hash);
-    if (jq)
-    {
-        JLOG(m_journal.debug()) << "checkAccept jq validation3 " << hash << " " << seq;
-    }
 
     if (!ledger)
     {
@@ -1072,31 +1041,12 @@ LedgerMaster::checkAccept(uint256 const& hash, std::uint32_t seq, bool jq)
 
         // FIXME: We may not want to fetch a ledger with just one
         // trusted validation
-        if (jq)
-        {
-            JLOG(m_journal.debug()) << "checkAccept jq validation4 " << hash << " " << seq;
-        }
         ledger = app_.getInboundLedgers().acquire(
-            hash, seq, InboundLedger::Reason::GENERIC, jq);
-        if (jq)
-        {
-            JLOG(m_journal.debug()) << "checkAccept jq validation5 " << hash << " " << seq;
-        }
+            hash, seq, InboundLedger::Reason::GENERIC);
     }
 
     if (ledger)
-    {
-        if (jq)
-        {
-            JLOG(m_journal.debug()) << "checkAccept jq validation6 " << hash << " " << seq;
-        }
         checkAccept(ledger);
-    }
-    if (jq)
-    {
-        JLOG(m_journal.debug()) << "checkAccept jq validation7"
-                                   " " << hash << " " << seq;
-    }
 }
 
 /**

diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp
@@ -2306,7 +2306,7 @@ NetworkOPsImp::recvValidation(
     JLOG(m_journal.trace())
         << "recvValidation " << val->getLedgerHash() << " from " << source;
 
-    handleNewValidation(app_, val, source, m_journal, true);
+    handleNewValidation(app_, val, source);
 
     pubValidation(val);
     std::stringstream ss;