fix: Improve concurrent path_find

[shortthefomo]

Improve concurrent path_find with shared AssetCache, incremental revalidation, parallel steady updates, and up to six paths without full-liquidity spare slots.

High Level Overview of Change

Make concurrent WebSocket path_find cheaper under load: shared trust-line cache, revalidate between full searches, parallel steady updates, up to 6 path alternatives (no full-liquidity spare).

Target: ~100 sessions, consensus FULL / load_factor ≈ 1, mean update gap ~ ledger close (under 4s).

Context of Change

Under many concurrent path_find sessions, most cost was trust-line reloads, full Pathfinder every update, and reprocessing everyone on new subscriptions.

Steady state is now cheaper (still best-effort):

• AssetCache — shared account line vectors, chunked load, soft reuse, session pins, budgets
• Revalidate — rippleCalculate on known paths; staggered full Pathfinder rediscovery
• Scheduling — serial first update; parallel steady revalidate; open ledger = new sessions only
• Path set — up to 6 ranked paths; drop last-path-must-fill / covering-path spare
• Ops — [path_find] knobs; get_counts pathfind cache counters

API Impact

• Public API: New feature (new methods and/or new fields)
• Public API: Breaking change (in general, breaking changes should only impact the next api_version)
• libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
• Peer protocol change (must be backward compatible or bump the peer protocol version)

Notes: request/response shapes unchanged. paths_computed may list up to 6 paths. get_counts counters are monitoring-only. No libxrpl / peer changes.

Before / After

Before	After
Trust-line thrash + full Pathfinder most updates	Shared cache + revalidate; full search when needed
New sessions reprocessed everyone	Serial create; parallel steady
~3 of 4 paths + spare/cover rules	Up to 6 ranked alternatives

Loadtest (100 concurrent WS path_find, ramp → observe → ramp-down):

Metric	Observed
server_state	FULL for entire run
load_factor	1.0 flat
Mean update gap	~500–800 ms (spikes ~2 s)
Throughput (observe)	~150–200 updates/s (peaks ~230)
Create latency	mostly <100 ms (occasional ~1 s spikes)
Cache	hits climb under load; reclaim to ~0 after last session

Test Plan

• xrpl.rpc.AssetCache — budget, advanceLedger, pins, concurrency
• xrpl.rpc.PathFindSub — revalidate, multi-session, 6-path, stagger, mid-close
• xrpl.app.Path / PathMPT
• Load harness: tools/pathfind-loadtest — ramp 100, FULL + counters
• Report: fix: Improve concurrent path_find #7962 (comment)

Future Tasks

• Mid-close revalidate if mean gap must be sub-close
• Ranking / auto-src tuning under multi-currency load

[shortthefomo]

Test Report of this PR code

Of note the test repeated between the 3.2.1 release and the PR code the path_find operations runs in each test are 1-1 and in the same order.

*Tests are run against the admin port so that throttling does not factor into the load.

Path find configuration used.

[path_search]
6

[path_search_fast]
2

[path_search_max]
8

Test tool used -> test-pathfind please use this tool to run your own tests.

*The report that was added here will be updated when this PR is reopened

[mvadari]

I had Claude do a review of this PR (collapsed here because it's fairly lengthy). It had some fairly serious concerns.

Overview

Reworks path_find for high-concurrency WS subscriptions:

• AssetCache becomes a long-lived, mutable, shared cache (shared_mutex, one full outgoing line vector per account, in-memory currency/direction filters, advanceLedger instead of rebuild-on-new-ledger, line budgets, hit/miss counters).
• PathRequest gains incremental revalidation (rippleCalculate on context_ paths, skipping Pathfinder) with staggered full rediscovery every ~20 closes.
• PathRequestManager::updateAll partitions into serial first-updates and parallel steady updates via std::async; lastIndex_ now pinned only on completion.
• Pathfinder drops the full-liquidity spare slot, last-path-must-fill rule, and covering-path retry; kMaxPaths 4 → 6; ranking capped at 200/80 candidates.
• New get_counts / insight counters; a gPathTable reordering for NonXrpToXrp.

The direction is sound and the diagnosis (trust-line reloads + full graph search on every close) is credible. But several changes look unsafe or regressive as written.

Correctness

1. Data race + potential dangling ReadView on AssetCache::ledger_ — blocking.
advanceLedger writes ledger_ under unique_lock, but getLedger() ([AssetCache.h:27](src/xrpld/rpc/detail/AssetCache.h:27)) returns std::shared_ptr const& and every consumer reads it without any lock: [PathRequest.cpp:627](src/xrpld/rpc/detail/PathRequest.cpp:627) (&*cache->getLedger()), the new revalidatePaths, [Pathfinder.cpp:230](src/xrpld/rpc/detail/Pathfinder.cpp:230), and updateAll itself. Previously this was safe because the cache was immutable after construction — a new ledger meant a new AssetCache. Now it's a concurrent read/write of a shared_ptr, and &*cache->getLedger() keeps no strong reference, so a replaced ledger whose last owner was ledger_ leaves PaymentSandbox over freed memory. With kPathSteadyUpdateParallelism = 8 plus any concurrent ripple_path_find job calling getAssetCache → advanceLedger, this is reachable.

getMPTs has the same shape and is worse: it returns std::shared_ptr<...> const& into mpts_, and advanceLedger does mpts_.clear(). A worker holding that reference gets a dangling reference (node destroyed), not just a stale one.

Minimum fix: have getLedger() return by value under a shared lock (and hold the shared_ptr for the lifetime of the sandbox), and make getMPTs return by value.

2. std::async for the parallel path — against project convention and unbounded.
std::async appears nowhere else in src/ — rippled schedules all concurrent work through JobQueue. runParallel creates up to 8 fresh OS threads per batch and blocks a job thread on .get(), so a 100-session ledger churns ~90+ thread creations per close, invisible to job-queue accounting and load shedding, with no isStopping() responsiveness inside a batch. This should be JobQueue::addJob + a completion latch, or the existing coroutine machinery.

3. Dropping the covering path is a behavior regression, not just a shape change.
The PR frames removal as "clients get more alternatives," but the deleted block at old [PathRequest.cpp:643](src/xrpld/rpc/detail/PathRequest.cpp:643) was the retry that rescued terNO_LINE / tecPATH_PARTIAL. Now if the maxPaths set can't deliver dstAmount, rc.result() != tesSUCCESS and the alternative is dropped entirely — for large payments over fragmented liquidity, clients get fewer usable alternatives (possibly none) where they previously got a working set. Removing last-path-must-fill and removing the covering path are separable; the first is defensible, the second needs justification or a measurement showing tecPATH_PARTIAL rates unchanged.

4. maxLines truncation silently returns wrong paths, and doesn't save the walk.
In loadOutgoingUnlocked, once totalLineCount_ approaches maxTotalLines_, maxLines collapses to the 64 floor — so a 5,000-line hub returns 64 arbitrary (owner-dir order) lines and pathfinding silently misses liquidity, with no log at warn and no signal to the client. Separately, in PathFindTrustLine::getItems the maxLines guard is a return inside the forEachItem lambda, which skips one entry rather than terminating the walk — the directory is still traversed in full, so the cap saves object construction only. Either wire a real early-exit or drop the pretense that it bounds walk cost.

5. authoritative is now ignored ((void)authoritative;).
Any newer ledger, including open ledgers, now advances the shared cache — so revalidatePaths/findPaths may run rippleCalculate against an open ledger where they previously required an authoritative one. That's a real semantic change buried in a (void) cast; either honor the flag or delete the parameter and explain why open-ledger pathfinding is acceptable.

6. Staleness window. kPathCacheReuseLedgers = 6 means trust lines up to ~24s old are used to compute source_amount against a newer ledger — a mixed-view result. Best-effort semantics arguably cover this, but 6 is a large default to ship without a config knob.

Performance

• The currency filter likely costs more than it saves in the hot loop. getRippleLines(acct, dir, currency) still materializes the full account vector, then allocates a new vector + shrink_to_fit and copies matches — on every addLink hop expansion. The old code scanned the same cached vector inline with continue, zero allocation. The comment "avoid materializing every IOU on large hubs" ([Pathfinder.cpp:779](src/xrpld/rpc/detail/Pathfinder.cpp:779)) describes something the code doesn't do. Consider returning a view/span + predicate, or an index by currency built once per account.
• The cache is never released. assetCache_ is a strong ref that is created once and only ever advanced, so a ReadView and up to 400k PathFindTrustLine objects are pinned forever, even with zero subscriptions — the previous weak_ptr freed them. ~AssetCache (and its stats log) is now effectively dead code. There's also no eviction when over budget; overBudget() is defined but never called.
• Sessions that keep failing (!bLastSuccess_) full-search on every close, which is exactly the expensive case the PR targets; 100 unroutable sessions see no improvement.

Style / conventions

• Large amounts of unrelated comment deletion in PathRequestManager.{h,cpp} (doc comments on updateAll, makePathRequest, makeLegacyPathRequest, insertPathRequest, the weak_ptr rationale). Please restore — it inflates the diff and loses context.
• Dead code: PathFindTrustLine::getItems' currency parameter has no caller passing it (AssetCache always passes nullopt); (void)fullLiquidityPath; and the fullLiquidityPath out-param kept "for call-site compatibility"; newRequests = false; inside if (!newRequests).
• rebuilds_ increments on every advanceLedger, not on rebuilds — misleading name for an ops-facing counter.
• static_cast<json::UInt> on std::uint64_t counters in GetCounts.cpp truncates at 2^32; these are cumulative and will wrap on a long-running server.
• Behavior comments assert properties the code doesn't guarantee (// Re-check after upgrade. in getOrLoadOutgoing — there is no re-check; loadOutgoingUnlocked does the freshness check, which is fine, but the comment is wrong in getOrLoadOutgoing and right in getMPTs).

Tests

No test changes at all for: incremental revalidation, staggered rediscovery, parallel steady updates, the 6-path shape, maxLines/budget truncation, or advanceLedger reuse-vs-force-clear. "xrpl.app.Path passes" only shows nothing regressed in single-threaded one-shot flows — it exercises none of the new machinery. At minimum I'd want:

• a unit test that a second update on the same ledger takes the revalidate path and produces the same alternatives as a full search;
• a test pinning advanceLedger retain vs. force-clear behavior and loadedSeq staleness;
• a concurrency test (N threads through one AssetCache) run under TSan — that would likely catch finding sjcl: undocumented build dependency: java #1;
• a case where the payment needs more than the best maxPaths can deliver, asserting the intended post-change response.

The load-test numbers are the core justification, but the harness isn't in the PR — please land it (or link it) so the claims are reproducible in CI.

Suggested split

This is ~1400 lines mixing five independent changes. I'd separate: (a) AssetCache restructure, (b) incremental revalidate + staggering, (c) parallel scheduling, (d) path-set shape / covering-path removal, (e) counters. (d) is a client-visible behavior change that deserves its own review and an API-CHANGELOG entry regardless of how the checkbox question is resolved; (c) needs the JobQueue rewrite; (a) needs the locking fix before anything else can be trusted.

[codecov]

Codecov Report

❌ Patch coverage is 77.95455% with 97 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/xrpld/rpc/detail/PathRequestManager.cpp	72.7%	44 Missing ⚠️
src/xrpld/rpc/detail/PathRequest.cpp	77.9%	25 Missing ⚠️
src/xrpld/rpc/detail/AssetCache.cpp	85.0%	15 Missing ⚠️
src/xrpld/rpc/detail/TrustLine.cpp	75.0%	5 Missing ⚠️
src/xrpld/rpc/handlers/admin/status/GetCounts.cpp	16.7%	5 Missing ⚠️
src/xrpld/rpc/detail/Pathfinder.cpp	90.0%	2 Missing ⚠️
src/xrpld/rpc/detail/PathRequest.h	0.0%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[github-actions]

This PR has conflicts, please resolve them in order for the PR to be reviewed.

[github-actions]

All conflicts have been resolved. Assigned reviewers can now start or resume their review.