Retry transient errors in ActivityCompletionClient.heartbeat - #2985
Open
arimu1 wants to merge 1 commit into
Open
Retry transient errors in ActivityCompletionClient.heartbeat#2985arimu1 wants to merge 1 commit into
arimu1 wants to merge 1 commit into
Conversation
…eartbeat recordHeartbeat was the only reply path that did not use GrpcRetryer, so a single RESOURCE_EXHAUSTED/UNAVAILABLE could fail an async activity heartbeat. Wrap the RPC with the same replyGrpcRetryerOptions used by complete, fail, and reportCancellation. Also rethrow ActivityCompletionException so cancel/ reset/paused signals are not wrapped as ActivityCompletionFailureException. Fixes temporalio#2984
|
|
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
2 participants
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.
What changed?
ManualActivityCompletionClientImpl.recordHeartbeatnow wraps the heartbeat RPC ingrpcRetryer.retryWithResult(...)with the samereplyGrpcRetryerOptionsused bycomplete,fail, andreportCancellation.Unit tests cover:
RESOURCE_EXHAUSTED/UNAVAILABLEuntil success (task-token and by-id paths)NOT_FOUNDActivityCanceledExceptionActivityCompletionFailureExceptionAlso rethrow
ActivityCompletionExceptionso cancel/reset/paused signals are not wrapped asActivityCompletionFailureExceptionby the outer catch.Why?
Async completion clients hold the task token outside the worker. A single transient server error (most often namespace rate limiting via
RESOURCE_EXHAUSTED, alsoUNAVAILABLE/DEADLINE_EXCEEDED) on heartbeat could fail the call immediately and risk activity heartbeat timeout. The other three reply RPCs already retry; heartbeat was the only exception.Breaking changes?
No intentional public API changes. Callers that previously observed cancel/reset/paused as
ActivityCompletionFailureExceptionwrapping the more specific type will now see the specific exception directly (ActivityCanceledException,ActivityResetException,ActivityPausedException).Server PR
N/A
Fixes #2984
Checklist