fix(workflow): use server-blocking WaitForInstance* RPCs instead of client polling#1839
fix(workflow): use server-blocking WaitForInstance* RPCs instead of client polling#1839paulstadler-mesh wants to merge 2 commits into
Conversation
…lient polling WaitForWorkflowStartAsync and WaitForWorkflowCompletionAsync polled GetInstance on a fixed 500ms/1s cadence. The sidecar already exposes server-blocking WaitForInstanceStart / WaitForInstanceCompletion RPCs that return the moment the instance reaches the target state, and the go-sdk (via durabletask-go) already uses them. This switches both methods to the blocking RPCs, wrapped in an exponential-backoff retry that re-issues on transient interruptions (DeadlineExceeded / Unavailable) and propagates cancellation, mirroring durabletask-go's client. Calls continue to flow through CreateCallOptions so the dapr-api-token is honored. Removes the now-dead client-poll tests and adds coverage for the blocking RPC path, transient-error retry, non-transient propagation, and cancellation. Fixes dapr#1838 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Stadler <paul.stadler@meshconnect.com>
There was a problem hiding this comment.
Pull request overview
This PR updates workflow wait operations to use the sidecar’s server-blocking WaitForInstanceStart and WaitForInstanceCompletion RPCs instead of client-side polling, reducing latency and sidecar load.
Changes:
- Replaced polling loops in workflow start/completion waits with blocking RPC calls.
- Added exponential backoff retries for transient wait interruptions.
- Updated unit tests to verify blocking RPC usage and retry behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/Dapr.Workflow/Client/WorkflowGrpcClient.cs |
Switches wait logic to blocking RPCs with retry handling. |
src/Dapr.Workflow/Logging.cs |
Adds debug logging for retrying interrupted wait RPCs. |
test/Dapr.Workflow.Test/Client/WorkflowGrpcClientTests.cs |
Updates wait tests from polling expectations to blocking RPC behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return await waitCall(request, grpcCallOptions); | ||
| } | ||
| catch (RpcException ex) when ( | ||
| !cancellationToken.IsCancellationRequested && | ||
| ex.StatusCode is StatusCode.DeadlineExceeded or StatusCode.Unavailable) | ||
| { | ||
| logger.LogWaitForInstanceRetry(ex, instanceId, ex.StatusCode, delay); | ||
| await Task.Delay(delay, cancellationToken); | ||
| delay = TimeSpan.FromMilliseconds( | ||
| Math.Min(delay.TotalMilliseconds * 2, MaxWaitRetryDelay.TotalMilliseconds)); |
There was a problem hiding this comment.
@paulstadler-mesh If you could please make this change, I'd appreciate it. If you're able to do this within the next 12 hours, I can get it into the Dapr GA release (aiming for Tuesday).
This is an excellent perf improvement - thank you for spotting it and submitting a PR!
|
@paulstadler-mesh I'm seeing some non-transient errors in the 1.17 and 1.18 RC runtimes regarding exception issues. Can you please review your changes and correct as necessary? If you're able to do this in the next 8 hours, I can get it into the GA release - otherwise, it'll have to wait for a patch release. |
|
Closing in favor of #1843 so we can get this into the next major release |
Description
WorkflowGrpcClient.WaitForWorkflowStartAsyncandWaitForWorkflowCompletionAsyncimplemented the wait as a client-side polling loop over the unaryGetInstanceRPC (500 ms / 1 s cadence) rather than calling the server-side blocking RPCs the sidecar already exposes for exactly this purpose:These return the moment the instance reaches the target state, so no client polling is needed. The generated
WaitForInstance*Asyncstubs existed but were never called.This PR switches both methods to the blocking RPCs, wrapped in an exponential-backoff retry that re-issues on transient interruptions (
DeadlineExceeded/Unavailable) and propagates cancellation — mirroring thedapr/durabletask-goclient (WaitForOrchestrationStart/WaitForOrchestrationCompletion), which is what the Dapr go-sdk uses. Calls continue to flow throughCreateCallOptions(...)so thedapr-api-tokenis honored.No public API change; behavior stays compatible (still returns
WorkflowMetadata, still throwsInvalidOperationExceptionif the instance doesn't exist).Issue reference
Fixes #1838
Impact / motivation
For short-lived workflows the fixed poll cadence dominated wall-clock latency. Local benchmark against Dapr 1.17.3 with a workflow that completes server-side in ~28 ms:
WaitForWorkflowCompletionAsync(poll)WaitForInstanceCompletionIt also removes needless repeated
GetInstanceload on the sidecar for long-running instances.go-sdk parity
dapr/durabletask-goclient/client_grpc.goalready does this — single blocking call wrapped in a backoff retry that stops on context cancellation. The .NET SDK was the lone outlier still client-polling. This brings it in line.How tested
WorkflowGrpcClientTeststo drive the blocking RPCs.dotnet test test/Dapr.Workflow.Test— 398 passing (net8.0 / net10.0).Checklist