Conversation
|
@copilot Please add unit tests to this PR that validate that calling DurableTaskGrpcClientFactory.getClient returns the same client instance for the same port, and that multiple clients can successfully be created for different ports. |
|
@sophiatev I've opened a new pull request, #257, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #254 by introducing a DurableTaskGrpcClientFactory that creates singleton gRPC clients per port, preventing repeated warnings about improperly closed gRPC channels. The previous implementation created a new client for each function invocation without properly closing them.
Changes:
- Added
DurableTaskGrpcClientFactoryto manage singleton client instances per port - Added a new package-private constructor to
DurableTaskGrpcClientthat accepts only a port parameter - Updated
DurableClientContextto use the factory instead of directly instantiating clients via the builder - Bumped version numbers across modules to 1.6.2
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClientFactory.java | New factory class that maintains singleton gRPC clients per port using ConcurrentHashMap |
| client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClient.java | Added new constructor accepting port parameter for use by the factory |
| azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java | Refactored to use factory instead of builder, simplified client caching logic |
| client/build.gradle | Version bump to 1.6.2 |
| azuremanaged/build.gradle | Version bump to 1.6.2 |
| azurefunctions/build.gradle | Version bump to 1.6.2 |
| CHANGELOG.md | Added entry for v1.6.2 referencing gRPC channel shutdown fix |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClient.java
Outdated
Show resolved
Hide resolved
client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClientFactory.java
Show resolved
Hide resolved
azurefunctions/src/main/java/com/microsoft/durabletask/azurefunctions/DurableClientContext.java
Outdated
Show resolved
Hide resolved
| public final class DurableTaskGrpcClientFactory { | ||
| private static final ConcurrentMap<Integer, DurableTaskGrpcClient> portToClientMap = new ConcurrentHashMap<>(); | ||
|
|
||
| public static DurableTaskClient getClient(int port) { | ||
| return portToClientMap.computeIfAbsent(port, DurableTaskGrpcClient::new); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
The factory creates singleton clients that are never closed during the application lifecycle. While this is intentional to prevent the channel shutdown warnings mentioned in issue #254, it means gRPC channels will remain open until the JVM exits. Consider adding a method to explicitly close all cached clients for scenarios where the application needs to shut down gracefully, such as when running in tests or when the Azure Functions host is shutting down. This would allow proper resource cleanup while maintaining the singleton pattern during normal operation.
client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClientFactory.java
Show resolved
Hide resolved
…nctions/DurableClientContext.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
client/src/main/java/com/microsoft/durabletask/DurableTaskGrpcClientFactory.java
Fixed
Show fixed
Hide fixed
…m/microsoft/durabletask-java into stevosyan/add-grpc-client-factory
* Initial plan * Add unit tests for DurableTaskGrpcClientFactory and fix compilation error Co-authored-by: sophiatev <38052607+sophiatev@users.noreply.github.com> * Remove changes to DurableTaskGrpcClient.java and proto file, keep only test file Co-authored-by: sophiatev <38052607+sophiatev@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: sophiatev <38052607+sophiatev@users.noreply.github.com>
Issue describing the changes in this PR
resolves #254
Currently, the
DurableClientContextinstantiates but never callscloseon itsDurableTaskGrpcClientinstance. This can lead to the errors referenced in #254. To remedy this, this PR instead changes theDurableClientContextto use theDurableTaskGrpcClientFactorywhich essentially creates static singletons of each gRPC client for each port throughout the application's lifetime. This should remove any issues with clients being recreated for the same port without properly being closed.Pull request checklist
CHANGELOG.md