Skip to content

fix: LoadManager::run() fee adjustment block outside while loop silently disables local fee protection #8038

Description

@rippleitinnz

Summary

The fee adjustment block in LoadManager::run() is placed outside the while loop. As a result, lowerLocalFee() and raiseLocalFee() are called exactly once - when stop_ is set during shutdown - rather than every second during normal operation.

The impact is broader than initially described: with localTxnLoadFee_ never changing, isLoadedLocal() was false for the node's entire lifetime, meaning every call site that sheds load based on that flag - path finding, the ledger RPC, peer drops, and the ledger cleaner — was silently disabled.

Bug identified by Nik Bougalis (@nbougalis).


Release Note (Operators)

Operators upgrading from any version affected by this bug should expect the following behaviour to return:

  • load_factor_local appearing in server_info during genuine job queue pressure
  • telINSUF_FEE_P rejections on transactions submitted to a node under high local load
  • Path finding, peer drops, and the ledger cleaner shedding load as designed under stress

This is correct behaviour being restored, not a regression.


Background

LoadManager::run() runs a dedicated thread that ticks every second. Its two responsibilities are:

  1. Stall detection — monitor the heartbeat timer and log/abort if the server stalls
  2. Local fee adjustment - call raiseLocalFee() when the job queue is overloaded, lowerLocalFee() otherwise, and call reportFeeChange() if the fee changed

The local fee (localTxnLoadFee_ in LoadFeeTrack) is a node-level protection mechanism independent of the open ledger fee escalation driven by TxQ. When a node's job queue is saturated, raiseLocalFee() increases localTxnLoadFee_, which causes Transactor::minimumFee() (via scaleFeeLoad()) to require a higher fee from incoming transactions. Transactions that do not meet the elevated minimum are rejected with telINSUF_FEE_P before consuming any network resources.

This mechanism is entirely separate from load_factor_fee_escalation, which responds to ledger capacity (transaction volume) and was not affected by this bug.


Root Cause

In src/xrpld/app/main/LoadManager.cpp, the fee adjustment block sits after the closing brace of the while loop:

void
LoadManager::run()
{
    while (true)
    {
        t += 1s;

        std::unique_lock sl(mutex_);
        if (cv_.wait_until(sl, t, [this] { return stop_; }))
            break;       // <-- exits loop on shutdown

        // Stall detection logic ...

    }   // <-- while loop ends here

    // Fee adjustment block — only reached after stop_ is set (shutdown)
    bool change = false;
    if (app_.getJobQueue().isOverloaded())
        change = app_.getFeeTrack().raiseLocalFee();
    else
        change = app_.getFeeTrack().lowerLocalFee();

    if (change)
        app_.getOPs().reportFeeChange();
}

cv_.wait_until() returns true when stop_ is set, causing break to exit the loop. The fee adjustment block is therefore only executed once - on shutdown - and never during normal operation.


Effect

  • load_factor_local never appears in server_info during normal operation
  • isLoadedLocal() is always false for the lifetime of the process
  • All shedding call sites gating on isLoadedLocal() - path finding, the ledger RPC, peer drops, the ledger cleaner - are silently off
  • raiseLocalFee() never fires regardless of job queue pressure
  • lowerLocalFee() never fires, so an elevated localTxnLoadFee_ is never decayed back to baseline
  • The node silently accepts work at base fee while under genuine stress, compounding the overload

Fix

Three changes in this PR:

1. LoadManager.cpp - move the fee adjustment block inside the while loop so it executes every tick.

2. NetworkOPs.cpp - dedicated mutex for lastFeeSummary_ - reportFeeChange() is called with masterMutex and peekMutex held (RCLConsensus.cpp:667), and pubServer() holds streamLock_ across the entire subscriber fan-out. Using streamLock_ in reportFeeChange() created a lock-ordering hazard where a ledger close could block on a slow subscriber broadcast. A dedicated feeSummaryMutex_ now guards lastFeeSummary_ exclusively, keeping the race fix without the ordering hazard.

3. NetworkOPs.cpp - first-subscriber reset - lastFeeSummary_ is reset when subServer() transitions from empty to non-empty, ensuring a newly subscribing client always receives a full serverStatus message carrying base_fee and load_factor_* fields.


Runtime Evidence

Two xrpld instances built from the same 3.3.0 source (00a178fb) on identical hardware (Intel i5-14600KF, Samsung 990 Pro NVMe), only the LoadManager.cpp fix applied to the patched binary. Both ran as validators on XRPL mainnet simultaneously.

Natural Event (production thresholds, no artificial load)

STOCK (unpatched): seq:106292670 load_factor:1
PATCHED (fixed): seq:106292670 load_factor:1.25 load_factor_local:1.25

Configured Event (lowered thresholds + CPU throttling)

STOCK (unpatched): tx (fee=10 drops): tesSUCCESS
PATCHED (fixed): tx (fee=10 drops): telINSUF_FEE_P

Unit Tests

LoadManager_test (6 cases) covers LoadFeeTrack mechanics and an integration test via jtx::Env:

Test case Description
raiseLocalFee requires two consecutive calls Verifies the raiseCount_ < 2 hysteresis guard
lowerLocalFee decays elevated fee back to baseline Confirms decay to kLftNormalFee (256)
lowerLocalFee at baseline returns false Confirms noop when already at floor
lowerLocalFee resets raiseCount Confirms raiseCount_ reset on lower
isLoadedLocal reflects fee state correctly Confirms isLoadedLocal() tracks fee and count state
LoadManager loop raises and decays load_factor_local jtx::Env test: raises fee twice past hysteresis, confirms decay to baseline. Fails on stock, passes with fix.

Subscribe_test race test covers the reportFeeChange data race fix.


Credit

Bug identified by Nik Bougalis (@nbougalis).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions