Fix workflow-monitor to support concurrent workflow monitoring#35
Open
FriederikeHanssen wants to merge 3 commits intomainfrom
Open
Fix workflow-monitor to support concurrent workflow monitoring#35FriederikeHanssen wants to merge 3 commits intomainfrom
FriederikeHanssen wants to merge 3 commits intomainfrom
Conversation
The workflow-monitor node had a critical design flaw preventing it from monitoring multiple workflows simultaneously. When multiple workflow messages arrived in quick succession, only the most recent workflow would be monitored, causing earlier workflows to be dropped from tracking. Root causes: - Single shared intervalId variable - new workflows cleared previous intervals - Captured message context in closures - new messages replaced old contexts - clearPolling() on every input - stopped all active monitoring Solution: - Replace single intervalId with Map-based tracking (activeWorkflows) - Each workflow gets independent polling with its own interval - Preserve message context (correlationId, _context, etc.) per workflow - Only clear polling for specific workflows when they reach terminal state - Show workflow count in status display when monitoring multiple workflows - If same workflowId is triggered twice, replace the old monitor Tests added: - Verify multiple workflows can be monitored independently - Verify message context is preserved for each workflow separately - Verify proper cleanup when same workflowId is re-triggered This enables use cases like batch workflow launching where multiple pipelines need to be monitored through completion simultaneously. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit addresses several critical issues in the concurrent workflow monitoring implementation:
- Fix memory leak: Store only essential workflow data (workspaceId, passthroughProps) instead of full message objects with potentially large payloads
- Optimize performance: Evaluate properties (workspaceId, pollInterval) once at workflow start rather than on every poll
- Improve robustness: Store local references in fetchStatus() to prevent edge cases if workflow is cleared during API calls
- Standardize error messages: Use consistent "Workflow {id}: {message}" format across all error paths
- Fix test: Update error handling test to match new error message format and prevent double-done calls
All 168 tests passing.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Changed from blacklist approach (excluding multiple specific properties) to only excluding payload. This makes the function more resilient to: - Future Node-RED properties (will automatically pass through) - Standard Node-RED properties like topic, _msgid (now preserved) - Custom user properties (all preserved except payload) The output message explicitly sets workflowId and payload, so those values are always controlled regardless of what comes through in passthrough props. This approach is simpler, more maintainable, and prevents future issues if Node-RED adds new internal properties. All 168 tests passing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
🐳 Docker PR Build
|
Contributor
|
Can we have a description of the problem this fixes and what it does please? I don't know what this is about 😅 |
Member
Author
|
@adamrtalbot sure I was still in tinkering mode. But essnetially: the Monitor Node didn't fire for second or third events when I had multiple pipeline chains in parallel. |
FriederikeHanssen
force-pushed
the
fix/workflow-monitor-concurrent-workflows
branch
from
cbb961e to
78454af
Compare
Member
Author
|
I moved some things to a different PR. To illustrate my problem better, here are two recordings when monitoring multiple workflows at the same time: old_monitor.movnew_monitor.mov |
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
2 participants
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
The workflow-monitor node could not track multiple workflows simultaneously. When a second workflow message arrived, it cancelled the first workflow's polling—making parallel pipeline monitoring impossible.
Root causes:
intervalId— new workflows cleared previous intervalsclearPolling()on every input — stopped all active monitoringSolution
Replace single interval with
Map<workflowId, WorkflowData>tracking:msg.topic,correlationId, custom props) stored per workflowImpact
msg.topiclost after first pollTesting