mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-04 06:52:47 +00:00
⚡ perf: Persist HITL checkpoints only on pause (lazy checkpointer) (#14024)
* ⚡ feat: Persist HITL checkpoints only on pause (skip clean-exit writes) With `durability: 'exit'` (set by the SDK whenever a checkpointer is active) LangGraph persists ONE checkpoint at the exit boundary on EVERY run — paused or not. So a non-paused HITL turn writes a dead checkpoint whose only fate is to be pruned by deleteAgentCheckpoint: pure write+delete churn on the common path, given HITL only ever resumes an *interrupt* checkpoint. `InterruptOnlyMongoSaver` (a MongoDBSaver subclass) persists only interrupt checkpoints and discards clean-exit ones, so a non-paused turn writes nothing. How it tells them apart (verified empirically against @langchain/langgraph, not docs): when a run interrupts, the runner calls `putWrites` with the `INTERRUPT` ("__interrupt__") channel for the checkpoint it's about to create, and that write's `config.checkpoint_id` equals the `checkpoint.id` of the `put` that immediately follows. A clean exit calls `put` with no preceding interrupt `putWrites`. So we record the checkpoint id of any interrupt `putWrites` and persist a `put` only when its `checkpoint.id` was so marked. Keying on the globally-unique checkpoint id (not thread_id) keeps this correct even when two runs race on the same conversation (the job-replacement scenario). Correctness is preserved end-to-end: interrupt checkpoints + their pending writes persist exactly as before (resume unchanged); clean checkpoints were only ever written-then-pruned, so not writing them is observationally equivalent. The eager prune stays as the backstop. Tests (mongodb-memory-server): a bare put() is discarded; an interrupt-seeded checkpoint is persisted with its __interrupt__ pending write; and an end-to-end real-graph run writes 0 checkpoints on a clean completion and a resumable one on interrupt. NOTE: a non-paused turn's deleteAgentCheckpoint now finds nothing to delete (a 0-match no-op) — a follow-up can skip that call entirely once the lingering-abandoned-pause cleanup role is reassigned to the TTL + expiry sweeper. * ⚡ feat: Drop the redundant clean-path checkpoint prune With the lazy checkpointer (InterruptOnlyMongoSaver) a non-paused turn no longer writes a clean-exit checkpoint, so the post-completion prune in chatCompletion's finally had nothing left to delete. It was also already redundant: every fresh turn runs a pre-run prune (`deleteAgentCheckpoint` before `processStream`) that clears any checkpoint orphaned by a prior abandoned pause — verified empirically that a lingering interrupt checkpoint WOULD otherwise poison a fresh turn (LangGraph continues the abandoned state + re-interrupts), and that the pre-run prune is what prevents it. The Mongo TTL remains the backstop, and the resume path still prunes after a successful finalize. Removing the clean-path prune also deletes its job-replacement race surface (round-17 F21): an older run's late finally can no longer delete a newer paused run's checkpoint, because there is no longer a clean-path prune to race. Dropped the now-dead F21 predicate test. Net per non-paused HITL turn: from {pre-run prune + checkpoint write + post-run prune} down to {pre-run prune} — no write, no post-completion delete. * 🛡️ fix: Anchor any pending-write checkpoint; stale-only eviction (Codex) Broaden the lazy saver's keep-rule from "interrupt-only" to "persist any checkpoint that carries pending writes" (renamed InterruptOnlyMongoSaver → LazyMongoSaver). This makes it robust to delta-channel graphs without changing behavior for LibreChat's graph: - K1 (P1): a delta-channel graph can write a synthetic PARENT/anchor checkpoint (no __interrupt__ mark) that the interrupt checkpoint then points at, with the delta writes stored under the parent id. The old rule discarded that parent, breaking delta-state resume. Now any checkpoint that received putWrites is persisted, so the anchor parent and its writes survive and resume can walk the chain. - K3 (P2): for the same reason, clean delta-write rows are no longer orphaned — their checkpoint is persisted alongside them. (For LibreChat's standard Annotation/messages graph a clean run makes no putWrites at all — verified empirically — so the common path still writes nothing and the optimization is unchanged.) - K2 (P2): the 1024 FIFO cap could evict a valid in-flight id whose put() was just behind Mongo I/O, mis-classifying its interrupt checkpoint as a clean exit. Replaced with time-based eviction: only ids older than 5 min (a put always follows its putWrites within ms) are swept; a recent in-flight id is never dropped, and the map grows rather than evict a valid id if nothing is stale. New integration test: a checkpoint anchored by a NON-interrupt write is persisted. Full agents/HITL suites green (108). * style(checkpointer): fix import order to satisfy sort-imports CI * fix(checkpointer): don't persist failed-turn (error-only) checkpoints LazyMongoSaver anchored on ANY pending write, so a non-paused turn that errors (LangGraph records an __error__ write then a put) was persisted and, with the clean-path prune removed, lingered until the next fresh-turn prune or the Mongo TTL. Anchor only on resumable writes — INTERRUPT or a real (non-__-prefixed) state/delta channel — so error/bookkeeping-only checkpoints are discarded at the source. Addresses Codex P3. Codex P2 (delta-stub parent orphan) is not reachable: the SDK graph uses standard Annotation/MessagesAnnotation channels (no DeltaChannel), and under durability:'exit' putWrites precedes put with a parentless boundary checkpoint — probe-confirmed against @langchain/langgraph@1.4. Documented the durability:'exit' invariant the saver depends on. Tests: error-only put discarded; e2e throwing graph persists 0 checkpoints. * fix(checkpointer): drop bookkeeping-only write batches, not just the checkpoint The prior fix stopped the failed-turn CHECKPOINT from persisting, but putWrites still forwarded the __error__ batch to MongoDBSaver.putWrites — writing a row to agent_checkpoint_writes whose parent checkpoint is then discarded. With the post-run deleteThread removed, that orphan row lingered until the Mongo TTL or the conversation's next pre-run prune. putWrites now drops a non-resumable (bookkeeping-only) batch entirely instead of forwarding it. Probed against a real MongoDBSaver (mongodb-memory-server): a throwing graph now leaves 0 checkpoints AND 0 write rows (was 0 + 1 orphan), while interrupt->resume is unaffected — the __interrupt__ write is resumable so it is still forwarded. Addresses Codex P2 (round 3). Tests: error-only put leaves no checkpoint and no write row; e2e throwing graph leaves both collections empty; new e2e interrupt->resume completes with the approval value. * fix(checkpointer): un-anchor a checkpoint whose putWrites failed; freshen comments Self-review findings on the converged PR: 1. LangGraph dispatches put() concurrently with putWrites (probe-confirmed on 1.4.5), and put() still completes when putWrites rejects — so a transient Mongo failure during the interrupt write could persist a checkpoint whose __interrupt__ row is missing (an unresumable phantom pause). putWrites now deletes the write anchor on rejection (best-effort) and rethrows, so that put() discards the checkpoint instead. The pre-recorded anchor stays where it is — recording after the await would drop slow-I/O interrupts on the success path, which the same probe showed is reachable. 2. Renamed leftovers: two comments still said InterruptOnlyMongoSaver; the class is LazyMongoSaver. 3. Documented why the pre-run prune is deliberately unconditional per HITL turn (any cheaper gate can go stale across replicas and skip the prune exactly when an orphaned interrupt exists). Test: failed putWrites → subsequent put persists nothing (14/14 green). * fix(checkpointer): bookkeeping write batches follow their checkpoint's fate The round-3 rule dropped bookkeeping-only putWrites batches (__error__/ __resume__/__no_writes__) unconditionally — batch-scoped, when the decision must be checkpoint-scoped. Probe-confirmed (langgraph 1.4.5, durability:'exit'): a Send fan-out that pauses on one sibling records the completed siblings as pure __no_writes__ batches on the RETAINED interrupt checkpoint; dropping those markers makes resume re-execute the completed siblings (side effects measured twice). Addresses Codex M2 (P2). putWrites now PARKS a bookkeeping-only batch in memory until the checkpoint's fate is known: forwarded when the checkpoint is anchored (or was just persisted — put is dispatched concurrently), dropped when put discards it. Net: an errored turn still leaves nothing durable (0 checkpoints, 0 write rows), and a retained checkpoint stores byte-for-byte what a plain MongoDBSaver would. Codex M1 (__resume__ lost on re-pause) did not reproduce: the re-pause emits [__interrupt__,__resume__] as ONE batch (anchored, forwarded whole) and a second resume on a rebuilt graph replays both answers correctly — but the fate-scoped buffering now covers a lone __resume__ batch in any ordering too. Tests: bookkeeping preserved on a retained checkpoint in either arrival order; e2e Send-sibling pause/resume with side-effect counters (was {a:2,c:2} under the drop rule, now {a:1,c:1}); error-only turn still leaves both collections empty. 16/16 green.
This commit is contained in:
parent
424ccffd83
commit
ed8547018c
6 changed files with 561 additions and 63 deletions
|
|
@ -337,32 +337,11 @@ describe('HITL Terminal-Side-Effect Guards', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('F21 — checkpoint prune is skipped when the generation was replaced', () => {
|
||||
// Mirrors client.js chatCompletion finally: prune only when NOT replaced.
|
||||
const shouldPrune = async ({ resumableStreamId, jobCreatedAt }) => {
|
||||
let replaced = false;
|
||||
if (resumableStreamId && jobCreatedAt != null) {
|
||||
const liveJob = await mockGenerationJobManager.getJob(resumableStreamId);
|
||||
replaced = !liveJob || liveJob.createdAt !== jobCreatedAt;
|
||||
}
|
||||
return !replaced;
|
||||
};
|
||||
|
||||
it('skips the prune when a newer job replaced this one', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue({ createdAt: 2000 });
|
||||
expect(await shouldPrune({ resumableStreamId: 'c1', jobCreatedAt: 1000 })).toBe(false);
|
||||
});
|
||||
|
||||
it('prunes when this is still the live job', async () => {
|
||||
mockGenerationJobManager.getJob.mockResolvedValue({ createdAt: 1000 });
|
||||
expect(await shouldPrune({ resumableStreamId: 'c1', jobCreatedAt: 1000 })).toBe(true);
|
||||
});
|
||||
|
||||
it('prunes without a lookup when there is no resumable stream id', async () => {
|
||||
expect(await shouldPrune({ resumableStreamId: undefined, jobCreatedAt: 1000 })).toBe(true);
|
||||
expect(mockGenerationJobManager.getJob).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
// (Removed: F21 — the chatCompletion clean-path checkpoint prune + its job-replacement
|
||||
// guard no longer exist. The lazy checkpointer never writes a clean-exit checkpoint, so
|
||||
// there is nothing to prune after a non-paused turn; the pre-run prune (before
|
||||
// processStream) clears any orphaned interrupt checkpoint instead. See
|
||||
// checkpointer.ts LazyMongoSaver and client.js chatCompletion.)
|
||||
|
||||
describe('F24 — resume catch-path terminal writes are skipped when replaced', () => {
|
||||
// Mirrors resume.js: stillLive gate around emitError/completeJob/deleteAgentCheckpoint.
|
||||
|
|
|
|||
|
|
@ -1615,6 +1615,10 @@ class AgentClient extends BaseClient {
|
|||
// conversation (one that expired or was aborted while paused) so this fresh
|
||||
// turn starts clean instead of rehydrating a stale interrupt — thread_id is
|
||||
// the stable conversationId. No-op when HITL is off or nothing is orphaned.
|
||||
// Deliberately UNCONDITIONAL per HITL turn: any cheaper gate (job metadata,
|
||||
// a Redis flag) can go stale across replicas/restarts and skip the prune
|
||||
// exactly when an orphan exists, while these are two indexed, usually-empty
|
||||
// deleteMany ops — correctness over a micro-optimization.
|
||||
if (streamId && isHITLEnabled(agentsEConfig?.toolApproval)) {
|
||||
await deleteAgentCheckpoint(this.conversationId, agentsEConfig?.checkpointer);
|
||||
}
|
||||
|
|
@ -1743,35 +1747,13 @@ class AgentClient extends BaseClient {
|
|||
this._resolveRun = null;
|
||||
}
|
||||
|
||||
// HITL: a turn that completed (or errored) without pausing leaves a dead
|
||||
// checkpoint. thread_id is the conversationId — stable across turns — so it
|
||||
// MUST be pruned before the next turn, or LangGraph would resume this turn's
|
||||
// state instead of starting fresh. Skip when paused (the checkpoint is needed
|
||||
// to resume) or when HITL is off (none was written). The Mongo TTL is the backstop.
|
||||
const agentsEConfig = appConfig?.endpoints?.[EModelEndpoint.agents];
|
||||
if (!this.pendingApproval && isHITLEnabled(agentsEConfig?.toolApproval)) {
|
||||
try {
|
||||
// Job-replacement guard: only prune if THIS generation is still the live job.
|
||||
// A newer request can replace this one on the same conversationId; if this
|
||||
// (older) run's finally lands after the newer run paused, pruning by
|
||||
// conversationId would delete the NEWER run's checkpoint and break its /resume.
|
||||
const resumableStreamId = this.options.req?._resumableStreamId;
|
||||
let replaced = false;
|
||||
if (resumableStreamId && this.jobCreatedAt != null) {
|
||||
const liveJob = await GenerationJobManager.getJobStore().getJob(resumableStreamId);
|
||||
replaced = !liveJob || liveJob.createdAt !== this.jobCreatedAt;
|
||||
}
|
||||
if (replaced) {
|
||||
logger.debug('[AgentClient] Skipping checkpoint prune — job was replaced', {
|
||||
streamId: resumableStreamId,
|
||||
});
|
||||
} else {
|
||||
await deleteAgentCheckpoint(this.conversationId, agentsEConfig?.checkpointer);
|
||||
}
|
||||
} catch (err) {
|
||||
logger.warn('[AgentClient] Failed to prune checkpoint after completion', err);
|
||||
}
|
||||
}
|
||||
// HITL: a non-paused turn deliberately prunes nothing here. The lazy checkpointer
|
||||
// (LazyMongoSaver) never persists a clean-exit checkpoint, so there is
|
||||
// nothing this turn left to delete. A checkpoint orphaned by a PRIOR abandoned pause
|
||||
// is cleared by the pre-run prune (before processStream) on the next turn, with the
|
||||
// Mongo TTL as the backstop. Dropping this post-completion prune also removes its
|
||||
// job-replacement race: an older run's late finally can no longer delete a newer
|
||||
// paused run's checkpoint, because there is no longer a clean-path prune to race.
|
||||
|
||||
run = null;
|
||||
config = null;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue