fix(graph): enforce branch_timeout_seconds and memory_conflict_strategy in parallel execution#6488
Draft
Jose37456 wants to merge 1 commit intoaden-hive:mainfrom
Draft
Conversation
…gy in parallel execution Root cause: ParallelExecutionConfig.branch_timeout_seconds and memory_conflict_strategy were declared but never read — asyncio.gather had no timeout wrapper and the output-merge loop unconditionally called write_async regardless of existing values. Fix 1 — branch timeout: wrap each execute_single_branch coroutine with asyncio.wait_for(timeout=config.branch_timeout_seconds) and switch gather to return_exceptions=True so a timeout in one branch doesn't cancel siblings. Timed-out branches are re-paired with their branch object and flow naturally into the existing on_branch_failure handling. Fix 2 — memory conflict strategy: before writing each output key check whether a value already exists and is different. If "first_wins", skip the write; if "error", raise RuntimeError; if "last_wins" (default), overwrite and log at DEBUG level. Fixes aden-hive#5706
PR Requirements WarningThis PR does not meet the contribution requirements. PR Author: @Jose37456 To fix:
Exception: To bypass this requirement, you can:
Micro-fix requirements (must meet ALL):
Why is this required? See #472 for details. |
github-actions
bot
added
the
pr-requirements-warning
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
1 participant
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.
Description
ParallelExecutionConfig.branch_timeout_secondsandmemory_conflict_strategywere declared but never read.asyncio.gatherhad no timeout wrapper, so a stalled branch could block the entire graph indefinitely. The output-merge loop unconditionally calledwrite_asyncon every key, making"first_wins"and"error"strategies no-ops.Type of Change
Related Issues
Fixes #5706
Changes Made
core/framework/graph/executor.py:_execute_parallel_branches():asyncio.wait_for(timeout=config.branch_timeout_seconds)and switchesgathertoreturn_exceptions=Trueso a timeout in one branch doesn't cancel siblings; timed-out branches flow into the existingon_branch_failurestrategy"first_wins"(skip),"error"(raiseRuntimeError), or"last_wins"(overwrite, log at DEBUG) accordinglyTesting
ruff checkclean)_execute_parallel_branches()"last_wins", 300s timeout)Checklist