fix(harness): bound hung memory background tasks without losing final flushes - #2947
Open
birdie7761 wants to merge 1 commit into
Open
fix(harness): bound hung memory background tasks without losing final flushes#2947birdie7761 wants to merge 1 commit into
birdie7761 wants to merge 1 commit into
Conversation
… flushes Since agentscope-ai#2777 the per-call memory flush and maintenance run detached from the agent response, tracked by the JVM-wide MemoryBackgroundTasks counter and drained by HarnessAgent.close() via awaitQuiescence. A hung model call, however, has no bound and no escape: - runFlush's pipeline has no timeout, so a hung flush keeps the conversation's coalescing slot in FLUSH_QUEUES running forever - every later flush of that conversation queues behind the dead slot and memory extraction stops for it; - MemoryBackgroundTasks.inFlight never returns to zero, so every subsequent awaitQuiescence call (e.g. HarnessAgent.close()) waits its full budget forever after; - maintenance consolidation ran consolidator.consolidate(rc).block() inside Mono.fromRunnable, so a hung consolidation leaked a boundedElastic worker for the rest of the process: the inner block subscription is unreachable from any outer dispose, and once the model eventually returned, the same worker would keep writing retention/prune results into the workspace. Fixes: - Both pipelines get a 5-minute timeout. The budget runs from subscription, so it also covers any boundedElastic pickup wait: under scheduler saturation a healthy-but-slow run may be skipped and logged - the safe direction for fire-and-forget work. This is the only cancellation mechanism: genuinely hung work is killed here. - doMaintenance becomes a reactive composition (expire -> consolidator.consolidate -> prune), so the pipeline timeout's cancel propagates down the chain into the consolidation model stream and the worker is actually freed; consolidation failures log and let the retention steps still run, matching the previous try/catch semantics. Deliberately NOT changed: quiescence waiting keeps its abandon semantics. Cancelling whatever is still in flight when close()'s 5s budget elapses would kill healthy flushes (an LLM call routinely outlives that budget, so with fire-and-forget memory work a close-during-flush is the common case, not the exception) and silently drop the last turn's memory extraction. Instead, an abandoned task keeps running and typically completes; only work hung past the pipeline timeout dies. MemoryBackgroundTasks is unchanged apart from a javadoc note documenting this division of responsibilities. New MemoryBackgroundHardeningTest: a short quiescence budget gives up without cancelling a healthy in-flight flush, which then completes on its own; a hung flush times out, releases the quiescence count and the conversation queue recovers; a hung consolidation times out and the cancellation is asserted to reach the consolidation subscription itself (the old .block() variant released the counter while the worker stayed stuck). Timeouts are injectable via package-private test hooks; production uses the 5-minute default.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Since #2777 the per-call memory flush and maintenance run fire-and-forget, but a hung model call has no bound and no escape:
runFlush's pipeline has no timeout, so a hung flush keeps the conversation's coalescing slot inFLUSH_QUEUESrunning forever — every later flush of that conversation queues behind the dead slot and memory extraction permanently stops for it;MemoryBackgroundTasks.inFlightnever returns to zero, so every subsequentclose()in the JVM waits its full 5s budget forever after;consolidator.consolidate(rc).block()insideMono.fromRunnable, so a hung consolidation leaks a boundedElastic worker for the rest of the process: the inner block subscription is unreachable from any outer dispose, and once the model eventually returns, the same worker keeps writing retention/prune results into the workspace.Fix
boundedElasticpickup wait): under scheduler saturation a healthy-but-slow run may be skipped and logged, the safe direction for fire-and-forget work.doMaintenancebecomes a reactive composition (expire → consolidator.consolidate → prune), so the timeout's cancel propagates down the chain into the consolidation model stream and the worker is actually freed. Consolidation failures log and let the retention steps still run, matching the previous try/catch semantics.Deliberately NOT changed: quiescence keeps its abandon semantics
Cancelling whatever is still in flight when
close()'s 5s budget elapses would kill healthy flushes: with the pipeline timeout in place, anything that survives to that moment is by definition still within its 5-minute bound, and an LLM flush routinely outlives 5s — with fire-and-forget memory work, a close-during-flush is the common case, not the exception. A close-time sweep would therefore routinely drop the last turn's memory extraction. Instead, an abandoned task keeps running and typically completes; only work hung past the pipeline timeout dies.MemoryBackgroundTasksis unchanged apart from a javadoc note documenting this division of responsibilities.Boundary
The #2935-style window of a background task writing against an already-torn-down workspace is not eliminated — it is bounded from unbounded down to at most 5 minutes (the pipeline timeout). That is the inherent cost of the abandon semantics, and strictly better than before this change.
Test evidence
New
MemoryBackgroundHardeningTest(3 cases): a short quiescence budget gives up without cancelling a healthy in-flight flush, which then completes on its own (the abandon invariant, locked); a hung flush times out, releases the quiescence count and the conversation queue recovers; a hung consolidation times out and the cancellation is asserted to reach the consolidation subscription itself (the old.block()variant released the counter while the worker stayed stuck). Timeouts are injectable via package-private test hooks; production uses the 5-minute default. Regression:MemoryFlushMiddlewareAsyncFlushTest/MemoryMaintenanceMiddlewareAsyncGateTestgreen; full harness suite 888/888.Checklist
mvn spotless:applymvn test)