Repository navigation
fix(server-utils): Cap the runs the LangChain callback handler keeps open - #25595
Conversation
…open `@langchain/core` never calls the end or error callback when a consumer stops reading a stream, so those runs stayed in the shared handler's map forever. The map is now an `LRUMap` capped at 1000 runs. An evicted run is dropped without ending its span, since ending it would send a span that lasted until eviction. Fixes #25510 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
size-limit report 📦Compared: 66e7c7a (PR test merge using develop at e13255f) Against: e13255f (develop) · base run #63743 ✅ Baseline matches: this diff contains only this PR's changes.
|
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 596ad0a. Configure here.
Optional E2E testsSome optional checks were not successful. They do not block the merge. 4 not successful, 61 successful. See the run.
Successful checks
|
isaacs
left a comment
isaacs
left a comment
There was a problem hiding this comment.
This is the smallest change that bounds memory: swap Map for LRUMap, reuse the 1000 cap that the other AI integrations use, and turn the ancestor walk into a single getAncestors() pass so that every ancestor gets an LRU touch. That refactor also lets getParentRunSpan and the conversation-id backfill share one walk, which reads a bit cleaner than the two loops before.
The open design question is what eviction should mean. This PR drops the span. The sibling integrations end it. Under steady leaks the map stays full, so eviction is probably not rare: it affects live runs whose last touch is older than the last 1000 run starts.
I would suggest we settle the drop-vs-end policy across the package, and consider a larger cap or dead-run-first eviction. This code itself is correct for what it sets out to do, and the tests guard the key behavior. 👍
|
|
||
| // Internal state - single instance tracks all spans | ||
| const runs = new Map<string, LangChainRun>(); | ||
| const runs = new LRUMap<string, LangChainRun>(MAX_TRACKED_LANGCHAIN_RUNS); |
There was a problem hiding this comment.
Flue, Mastra, and pi-durable all use the same 1000 cap, but each one ends the oldest span before eviction. Their comments call out that LRUMap.set would otherwise drop it unended. This PR deliberately does the opposite, with the reason "ending it would send a span that lasted until eviction".
Ending on eviction gives a truncated span with wrong duration, but keeps the tree whole. Either policy can be right, but the package should pick one, so users of different AI integrations don't see different failures under the same pressure.
Suggestion: either follow the sibling pattern (check runs.size >= MAX_TRACKED_LANGCHAIN_RUNS, remove the oldest key and end() it before set), or, if the we decide we prefer dropping, say so in the PR body and open a follow-up to align Flue, Mastra and pi-durable. At minimum, explain in the PR why LangChain differs.
There was a problem hiding this comment.
you're right, I'd misread mastra (looked at the span registry, not _trackSpan). switched to ending the evicted run like flue/mastra/pi-durable. so the package has one policy, and an evicted live run stays in the tree with a cut-off duration instead of disappearing.
| } | ||
|
|
||
| const run: LangChainRun = { span, parentRunId, activeSpan, conversationId }; | ||
| runs.set(runId, run); |
There was a problem hiding this comment.
This can drop live runs ahead of leaked ones. Leaked runs are never removed except by eviction. In a process that leaks steadily, the map reaches 1000 entries and stays full.
From then on, every runs.set() evicts the least recently touched entry. That is a leaked run only while all leaked runs are older than all live ones. Leaks that happen after a live run's last touch are newer than it, so the live run goes first.
A run is touched only when it starts, when a descendant starts, and in its own end/error handler. There is no handleLLMNewToken handler, so a chat model streaming for a long time with no child runs is not touched. On a busy server with a saturated map, the effective rule becomes "a run survives only if it was touched within the last 1000 run starts across the process." For example, one 60s streamed chat call, while other requests start 20 runs per second, would be evicted after about 50s and silently lost.
1000 open runs also bounds legitimate concurrency. Each in-flight agent request holds its current path open (often 3 to 6 runs), so a few hundred concurrent requests can push live runs out even with no leaks.
Suggestion: consider one or more of these.
- Prefer evicting runs that are known to be dead. For example, before
seton a full map, drop entries whose root span (getRootSpan(run.span)) is no longer recording. An abandoned stream inside a finished request then goes before a live run. This was also one of the options suggested in LangChain callback handler keeps runs that never end in memory #25510, but it's a bit more complicated of course. - Raise the cap maybe? Entries are fairly small, and the goal is to bound a leak, not to keep memory tight. 10,000 might still be a fine.
- Emit a
DEBUG_BUILDdebug.warnwhen the cap is hit, so a user who sees missing LangChain spans has some signal.
Or, if the we go with the current tradeoff, say so in the PR body so the decision is explicit.
There was a problem hiding this comment.
confirmed, nothing touches a streaming chat run. added a one time debug.warn when the map first fills, so missing/cut-short spans have a signal. kept the cap at 1000 to match the siblings. dead-run-first eviction doesn't help leaked runs that are their own root, so I'd rather do it package wide if we want it. noted the tradeoff in the description.
…rations Flue, Mastra and pi-durable all end the span they evict, so LangChain now does too instead of dropping it unended. A debug warning fires the first time the run map fills. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@langchain/corenever calls the end or error callback when a consumer stops reading a stream. The handler is shared across the process, so those runs stay in its map forever, each keeping its request's span tree in memory.The map is now an
LRUMapcapped at 1000 runs, and like Flue, Mastra and pi-durable, an evicted run's span is ended so the tree stays whole. A run is only marked as used when it or a descendant starts, so once leaks fill the map, a long streaming call can be ended early with a cut-off duration. A debug warning fires the first time the map fills.Fixes #25510
🤖 Generated with Claude Code