-
Notifications
You must be signed in to change notification settings - Fork 1.3k
fix(graph): fix llmnode thread safety #2511
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?
Conversation
This way to slove one node,will change it |
94b6b4e
to
876901a
Compare
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.
Pull Request Overview
This PR fixes thread safety issues in the CompiledGraph
class by changing from storing pre-created node instances to storing node factory functions. This ensures that each request gets a fresh instance, preventing shared state issues in concurrent environments.
Key changes:
- Replace the
nodes
map storingAsyncNodeActionWithConfig
instances withnodeFactories
map storingNode.ActionFactory
functions - Update all references to create new instances on-demand using the factories
- Add comprehensive test coverage for concurrent node creation scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
CompiledGraph.java | Core thread safety fix - replaces instance storage with factory pattern |
NodeFactoryPatternTest.java | New test suite verifying factory pattern and concurrent access behavior |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
spring-ai-alibaba-graph-core/src/main/java/com/alibaba/cloud/ai/graph/CompiledGraph.java
Outdated
Show resolved
Hide resolved
spring-ai-alibaba-graph-core/src/main/java/com/alibaba/cloud/ai/graph/CompiledGraph.java
Outdated
Show resolved
Hide resolved
…i/graph/CompiledGraph.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…i/graph/CompiledGraph.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
spring-ai-alibaba-graph-core/src/main/java/com/alibaba/cloud/ai/graph/CompiledGraph.java
Show resolved
Hide resolved
spring-ai-alibaba-graph-core/src/main/java/com/alibaba/cloud/ai/graph/CompiledGraph.java
Outdated
Show resolved
Hide resolved
spring-ai-alibaba-graph-core/src/main/java/com/alibaba/cloud/ai/graph/CompiledGraph.java
Outdated
Show resolved
Hide resolved
…i/graph/CompiledGraph.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…i/graph/CompiledGraph.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Describe what this PR does / why we need it
fix llmnode thread safety the message is thread unsafe
Does this pull request fix one issue?
Close #2510
Describe how you did it
Create copies of local variables to handle state
Describe how to verify it
Special notes for reviews