Follow-ups from pve-agent3's review of #246 (perf: per-candidate SQL removed from /paths and /hop_analytics). Details are in the review comment on #246.
-
A stale resolved-path LRU can change results. The old pre-filter read the current DB row. The canonical path now comes from fetchResolvedPathForTxBest, which serves from apiResolvedPathLRU first, and nothing invalidates that LRU when the stored path changes (lruDelete has no non-test callers). If the ingestor's observation upsert replaces a stored resolved_path, /paths can confirm or reject a candidate from stale data until the entry is evicted. Decide whether to:
- invalidate on change, for example in the poll loop that sees updated observations; or
- bound the entry age; or
- document the trade-off.
Add a test either way.
-
Efficiency. Candidates that only the old SQL check rejected (hash collisions, stale index entries) now cost a canonical-path fetch each. Each such fetch also adds an LRU entry for a transmission that does not belong to the node. Consider not caching the result for rejected candidates, or measure it.
-
Tests. TestPathLenFast_* pass trivially without a fast path. Two of the new tests fail on master only through their query-count assertions. Make that explicit in the test names or comments, and add a behavioural assertion that distinguishes before from after where possible.
Follow-ups from pve-agent3's review of #246 (perf: per-candidate SQL removed from
/pathsand/hop_analytics). Details are in the review comment on #246.A stale resolved-path LRU can change results. The old pre-filter read the current DB row. The canonical path now comes from
fetchResolvedPathForTxBest, which serves fromapiResolvedPathLRUfirst, and nothing invalidates that LRU when the stored path changes (lruDeletehas no non-test callers). If the ingestor's observation upsert replaces a storedresolved_path,/pathscan confirm or reject a candidate from stale data until the entry is evicted. Decide whether to:Add a test either way.
Efficiency. Candidates that only the old SQL check rejected (hash collisions, stale index entries) now cost a canonical-path fetch each. Each such fetch also adds an LRU entry for a transmission that does not belong to the node. Consider not caching the result for rejected candidates, or measure it.
Tests.
TestPathLenFast_*pass trivially without a fast path. Two of the new tests fail on master only through their query-count assertions. Make that explicit in the test names or comments, and add a behavioural assertion that distinguishes before from after where possible.