-
Notifications
You must be signed in to change notification settings - Fork 3.1k
fix: prevent memory exhaustion in loops with bounded iteration outputs #2527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
fix: prevent memory exhaustion in loops with bounded iteration outputs #2527
Conversation
… fixes, subflow resize clamping
…ribe, auth checks, new db indexes
…dioai#2481) The realtime service network policy was missing the custom egress rules section that allows configuration of additional egress rules via values.yaml. This caused the realtime pods to be unable to connect to external databases (e.g., PostgreSQL on port 5432) when using external database configurations. The app network policy already had this section, but the realtime network policy was missing it, creating an inconsistency and preventing the realtime service from accessing external databases configured via networkPolicy.egress values. This fix adds the same custom egress rules template section to the realtime network policy, matching the app network policy behavior and allowing users to configure database connectivity via values.yaml.
…rovements, additional kb tag types
|
@aadamsx is attempting to deploy a commit to the Sim Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR implements memory management for long-running workflow loops by adding bounded storage for iteration outputs and block logs. Key Changes:
Considerations:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant WF as Workflow Executor
participant BE as BlockExecutor
participant LO as LoopOrchestrator
participant LS as LoopScope
participant CTX as ExecutionContext
Note over WF,CTX: Loop Execution with Memory Management
WF->>LO: initializeLoopScope(ctx, loopId)
LO->>LS: create LoopScope
LS-->>LO: scope with empty allIterationOutputs[]
loop Each Iteration
WF->>BE: execute(ctx, node, block)
BE->>BE: createBlockLog()
BE->>BE: addBlockLogWithMemoryLimit(ctx, blockLog)
alt blockLogs.length > MAX_BLOCK_LOGS (500)
BE->>CTX: slice and discard oldest logs
BE->>BE: log warning
end
alt blockLogs.length % 50 === 0
BE->>BE: estimateBlockLogsSize()
alt size > MAX_BLOCK_LOGS_SIZE_BYTES (100MB)
BE->>CTX: discard oldest half of logs
BE->>BE: log warning
end
end
BE->>CTX: blockLogs.push(blockLog)
BE-->>WF: NormalizedBlockOutput
WF->>LO: storeLoopNodeOutput(ctx, loopId, nodeId, output)
LO->>LS: currentIterationOutputs.set(nodeId, output)
WF->>LO: evaluateLoopContinuation(ctx, loopId)
LO->>LO: collect iterationResults from currentIterationOutputs
LO->>LO: addIterationOutputsWithMemoryLimit(scope, results)
alt allIterationOutputs.length > MAX_STORED_ITERATION_OUTPUTS (100)
LO->>LS: slice and discard oldest iterations
LO->>LO: log warning
end
alt allIterationOutputs.length % 10 === 0
LO->>LO: estimateObjectSize()
alt size > MAX_ITERATION_OUTPUTS_SIZE_BYTES (50MB)
LO->>LS: discard oldest half of iterations
LO->>LO: log warning
end
end
LO->>LS: allIterationOutputs.push(results)
LO->>LS: currentIterationOutputs.clear()
LO->>LO: evaluateCondition()
alt condition false
LO->>LO: createExitResult()
LO-->>WF: shouldExit=true
else condition true
LO->>LS: increment iteration
LO-->>WF: shouldContinue=true
end
end
Note over WF,CTX: Memory bounded to ~150MB total
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (5)
-
apps/sim/executor/orchestrators/loop.ts, line 519-529 (link)logic:
JSON.stringify()runs on EVERY iteration before size check, defeating the purpose of memory optimization. With 1000 iterations, this serializes potentially GBs of data 1000 times.Move size check to only run periodically (e.g., every 10 iterations) like
block-executor.ts:Then in
addIterationOutputsWithMemoryLimitat line 497:// Check memory size limit periodically (every 10 iterations to avoid frequent serialization) if (scope.allIterationOutputs.length % 10 === 0) { const estimatedSize = this.estimateObjectSize(scope.allIterationOutputs) if (estimatedSize > DEFAULTS.MAX_ITERATION_OUTPUTS_SIZE_BYTES) {
-
apps/sim/executor/execution/block-executor.ts, line 704-710 (link)logic: returns max limit on error, which prevents cleanup and allows unbounded growth if serialization consistently fails
-
apps/sim/executor/orchestrators/loop.ts, line 519-529 (link)logic: same issue - returning max limit prevents cleanup on serialization errors
-
apps/sim/executor/orchestrators/loop.ts, line 483-494 (link)logic: slicing from
discardCountremoves NEWEST iterations instead of oldest. The intent is to keep the most recent 100 iterations. -
apps/sim/executor/execution/block-executor.ts, line 673-682 (link)logic: slicing from
discardCountremoves NEWEST logs instead of oldest
3 files reviewed, 5 comments
This change addresses memory exhaustion issues that occur when running workflows with loops containing agent blocks that make many tool calls. Problem: Memory accumulated unbounded in two key areas: 1. allIterationOutputs in LoopScope - every iteration pushed results 2. blockLogs in ExecutionContext - every block execution added logs Solution: Added memory management with configurable limits in constants.ts: - MAX_STORED_ITERATION_OUTPUTS (100) and MAX_ITERATION_OUTPUTS_SIZE_BYTES (50MB) - MAX_BLOCK_LOGS (500) and MAX_BLOCK_LOGS_SIZE_BYTES (100MB) Loop orchestrator (loop.ts): - New addIterationOutputsWithMemoryLimit() method - Periodic size checks (every 10 iterations) to avoid serialization overhead - Discards oldest iterations when limits exceeded Block executor (block-executor.ts): - New addBlockLogWithMemoryLimit() method - Periodic size checks (every 50 logs) - Discards oldest logs when limits exceeded Trade-offs: - Final aggregated results contain only recent iterations - Logs show warning when truncation occurs for debugging Fixes simstudioai#2525
36bbe99 to
387efb6
Compare
Response to Greptile ReviewThe issues identified in the review on commit aae7c40 have been addressed in the current HEAD (387efb6): ✅ Performance Issue (Fixed)Added periodic size checking - ✅ Error Handling (Fixed)Changed error return to ℹ️ Array Slicing Logic (Already Correct)The Greptile comment about array slicing is incorrect. The current logic is working as intended: scope.allIterationOutputs = scope.allIterationOutputs.slice(discardCount)When we have 110 items and MAX is 100:
This correctly implements "discard oldest, keep newest" as stated in the docstring. The array is built by pushing new items, so index 0 is oldest and the last index is newest. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Additional Comments (1)
-
apps/sim/executor/orchestrators/loop.ts, line 496-497 (link)logic: memory check happens only on exact multiples of 10, missing iterations 101-109 after first truncation at iteration 100
if iteration 100 hits count limit and truncates to 100 items, then iteration 101-109 won't check memory size until iteration 110, potentially allowing another 50MB+ growth
4 files reviewed, 1 comment
Summary
This PR addresses memory exhaustion issues that occur when running workflows with loops containing agent blocks that make many tool calls (e.g., MCP file operations).
Fixes #2525
Problem
Memory accumulated unbounded in two key areas during workflow execution:
allIterationOutputsin LoopScope - every loop iteration pushed results to this array with no limitblockLogsin ExecutionContext - every block execution added logs with no pruningThis caused OOM crashes on systems with 64GB+ RAM during long-running workflow executions with loops.
Solution
Added memory management with configurable limits in two places:
Loop Orchestrator (
apps/sim/executor/orchestrators/loop.ts)addIterationOutputsWithMemoryLimit()methodMAX_STORED_ITERATION_OUTPUTS(default: 100)MAX_ITERATION_OUTPUTS_SIZE_BYTES(default: 50MB)Block Executor (
apps/sim/executor/execution/block-executor.ts)addBlockLogWithMemoryLimit()methodMAX_BLOCK_LOGS(default: 500)MAX_BLOCK_LOGS_SIZE_BYTES(default: 100MB)New Constants (
apps/sim/executor/constants.ts)MAX_STORED_ITERATION_OUTPUTS: 100MAX_ITERATION_OUTPUTS_SIZE_BYTES: 50MBMAX_BLOCK_LOGS: 500MAX_BLOCK_LOGS_SIZE_BYTES: 100MBTrade-offs
loop.resultswill contain only the most recent iterations (up to 100)Testing
Files Changed
apps/sim/executor/constants.ts- Added new configurable limitsapps/sim/executor/orchestrators/loop.ts- Added memory-bounded iteration storageapps/sim/executor/execution/block-executor.ts- Added memory-bounded log storage