-
Notifications
You must be signed in to change notification settings - Fork 127
perf: reuse executors and share resources across flow runs #674
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -990,11 +990,10 @@ def execute_with_retry(self, input_data: dict[str, Any] | BaseModel, config: Run | |
| error = None | ||
| n_attempt = self.error_handling.max_retries + 1 | ||
| executor = None | ||
| timed_out = False | ||
|
|
||
| try: | ||
| if timeout is not None: | ||
| executor = ContextAwareThreadPoolExecutor() | ||
| executor = ContextAwareThreadPoolExecutor(max_workers=1) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single-worker timeout executor blocks retries after timeoutHigh Severity The timeout executor changed from default Additional Locations (1) |
||
|
|
||
| for attempt in range(n_attempt): | ||
| merged_kwargs = merge(kwargs, {"execution_run_id": uuid4()}) | ||
|
|
@@ -1032,7 +1031,6 @@ def execute_with_retry(self, input_data: dict[str, Any] | BaseModel, config: Run | |
| return output | ||
| except TimeoutError as e: | ||
| error = e | ||
| timed_out = True | ||
| self.run_on_node_execute_error(config.callbacks, error, **merged_kwargs) | ||
| logger.warning(f"Node {self.name} - {self.id}: timeout.") | ||
| except Exception as e: | ||
|
|
@@ -1052,9 +1050,7 @@ def execute_with_retry(self, input_data: dict[str, Any] | BaseModel, config: Run | |
| raise error | ||
| finally: | ||
| if executor is not None: | ||
| # Use cancel_futures=True and wait=False when timeout occurred to prevent | ||
| # blocking on threads that may be stuck waiting (e.g., on input_queue.get()) | ||
| executor.shutdown(wait=not timed_out, cancel_futures=timed_out) | ||
| executor.shutdown(wait=False) | ||
|
|
||
| def execute_with_timeout( | ||
| self, | ||
|
|
@@ -1087,8 +1083,10 @@ def execute_with_timeout( | |
| return future.result(timeout=timeout) | ||
| except TimeoutError: | ||
| # Cancel the future to prevent further execution if possible. | ||
| # Note: cancel() only works if the task hasn't started yet. | ||
| # For running tasks, we rely on executor.shutdown(cancel_futures=True). | ||
| # Note: cancel() only prevents tasks that have not started yet. | ||
| # Already-running tasks will continue until they complete; this is | ||
| # a Python threading limitation. The per-node executor is shut | ||
| # down (wait=False) in the finally block of execute_with_retry. | ||
| future.cancel() | ||
| raise | ||
|
|
||
|
|
||


Uh oh!
There was an error while loading. Please reload this page.