diff --git a/docker/telemetry/grafana/dashboards/ledger-sync-health.json b/docker/telemetry/grafana/dashboards/ledger-sync-health.json index 65a866dfd8..c284dcb596 100644 --- a/docker/telemetry/grafana/dashboards/ledger-sync-health.json +++ b/docker/telemetry/grafana/dashboards/ledger-sync-health.json @@ -1729,7 +1729,7 @@ "type": "prometheus", "uid": "${DS_PROMETHEUS}" }, - "expr": "label_replace(label_join(label_replace(clamp_max(sync_state{service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\", xrpl_network_type=~\"$xrpl_network_type\", service_name=~\"$service_name\", xrpl_work_item=~\"$xrpl_work_item\", xrpl_branch=~\"$xrpl_branch\", xrpl_node_role=~\"$xrpl_node_role\", metric=\"ledgers_behind\"}, 1000000), \"series\", \"Ledgers Behind\", \"\", \"\"), \"xrpl_ident\", \", \", \"service_instance_id\", \"xrpl_branch\", \"xrpl_work_item\"), \"xrpl_ident\", \"[$1]\", \"xrpl_ident\", \"(?:, )*(.*[^, ])(?:, )*\")", + "expr": "label_replace(label_join(label_replace(sync_state{service_instance_id=~\"$node\", deployment_environment=~\"$deployment_environment\", xrpl_network_type=~\"$xrpl_network_type\", service_name=~\"$service_name\", xrpl_work_item=~\"$xrpl_work_item\", xrpl_branch=~\"$xrpl_branch\", xrpl_node_role=~\"$xrpl_node_role\", metric=\"ledgers_behind\"}, \"series\", \"Ledgers Behind\", \"\", \"\"), \"xrpl_ident\", \", \", \"service_instance_id\", \"xrpl_branch\", \"xrpl_work_item\"), \"xrpl_ident\", \"[$1]\", \"xrpl_ident\", \"(?:, )*(.*[^, ])(?:, )*\")", "refId": "A" } ], diff --git a/src/xrpld/app/ledger/InboundLedger.h b/src/xrpld/app/ledger/InboundLedger.h index 96e84724a0..7543295338 100644 --- a/src/xrpld/app/ledger/InboundLedger.h +++ b/src/xrpld/app/ledger/InboundLedger.h @@ -113,16 +113,37 @@ public: void runData(); + /** + * Mark this acquire as still alive, so the sweeper does not reclaim it. + * + * InboundLedgers::sweep() destroys any acquire whose last action is more + * than a minute old, and that destruction is what the telemetry reports as + * `outcome=abandoned`. Anything that represents real progress must therefore + * call this, or a fetch that is working normally can be deleted for looking + * idle. + * + * @note Thread-safe and lock-free: a relaxed store of the clock's tick + * count. Called from peer threads on the receive path, from the + * acquiring thread, and read by the sweeper on the timer thread, so it + * cannot be a plain member. + */ void touch() { - lastAction_ = clock_.now(); + lastAction_.store(clock_.now().time_since_epoch().count(), std::memory_order_relaxed); } + /** + * When this acquire last made progress, for the sweeper's age check. + * + * @note Thread-safe and lock-free: a relaxed load. A value one tick stale is + * acceptable against a 60-second sweep interval. + */ clock_type::time_point getLastAction() const { - return lastAction_; + return clock_type::time_point{ + clock_type::duration{lastAction_.load(std::memory_order_relaxed)}}; } /** @@ -354,7 +375,17 @@ private: std::optional missingNodes) noexcept; clock_type& clock_; - clock_type::time_point lastAction_; + + /** + * Tick count of the last action, as the clock's duration rep. + * + * Stored as the raw rep rather than a `time_point` so it can be atomic: the + * receive path writes it from peer threads while InboundLedgers::sweep() + * reads it from the timer thread. Relaxed on both sides -- the sweeper + * compares against a 60-second threshold, so a value one tick out of date + * cannot change its decision. + */ + std::atomic lastAction_; std::shared_ptr ledger_; bool haveHeader_{false}; diff --git a/src/xrpld/app/ledger/detail/InboundLedger.cpp b/src/xrpld/app/ledger/detail/InboundLedger.cpp index 983315e471..bc9e5b8ad0 100644 --- a/src/xrpld/app/ledger/detail/InboundLedger.cpp +++ b/src/xrpld/app/ledger/detail/InboundLedger.cpp @@ -1430,6 +1430,15 @@ InboundLedger::gotData( // Mirror the depth for the telemetry gauge, which must not take this lock. receivedDataDepth_.store(receivedData_.size(), std::memory_order_relaxed); + // A peer just answered, so this acquire is making progress even if its turn + // to apply the data has not come up yet. Without this the sweeper's one + // minute idle test measures the wait for a JtLedgerData slot rather than + // real inactivity, and deletes fetches that are still being served: on a + // fresh mainnet sync that produced 490 abandoned acquires against zero + // expired retry budgets, because only the constructor, update() and done() + // ever refreshed the timestamp. + touch(); + if (receiveDispatched_) return false; diff --git a/src/xrpld/app/misc/NetworkOPs.cpp b/src/xrpld/app/misc/NetworkOPs.cpp index b331c1229d..0b233aee7b 100644 --- a/src/xrpld/app/misc/NetworkOPs.cpp +++ b/src/xrpld/app/misc/NetworkOPs.cpp @@ -1055,6 +1055,16 @@ NetworkOPsImp::getLedgersBehindNetwork() const auto const validated = registry_.get().getLedgerMaster().getValidLedgerIndex(); + // A node that has validated nothing is not "behind" by the whole sequence + // space; the distance is undefined until there is a validated ledger to + // measure from. Returning the raw difference here reported ~105.9 million on + // a fresh mainnet start, which is a real reading of a meaningless quantity: + // it auto-scaled every consumer's axis and would trip any threshold. Report + // zero until the first ledger is validated, and let the sync-state signals + // say how far along the initial acquire is. + if (validated == 0) + return 0; + // Floor at zero: we can legitimately be ahead of every peer's reported // range, and a peer that has reported nothing yet leaves the target at 0. return networkTarget > validated ? networkTarget - validated : 0;