Implement timer origins and backwards-compatible optional timers#1790
Implement timer origins and backwards-compatible optional timers#1790
Conversation
…vent Add timer origin protobuf messages (TimerOriginCreateTimer, TimerOriginExternalEvent, TimerOriginActivityRetry, TimerOriginChildWorkflowRetry) and origin oneof fields to CreateTimerAction and TimerCreatedEvent. Implement origin assignment in WorkflowOrchestrationContext: - CreateTimer sets TimerOriginCreateTimer - WaitForExternalEvent sets TimerOriginExternalEvent (with sentinel for indefinite waits) - Activity retry timers set TimerOriginActivityRetry with stable taskExecutionId - Child workflow retry timers set TimerOriginChildWorkflowRetry with first-child instanceId Add drop-and-shift replay tolerance for optional external event timers when replaying pre-patch histories that lack the synthetic timer. Implements #1787 Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/d91a8228-5499-4c7f-bf21-c9bd4fd6147a Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
… compatibility Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/d91a8228-5499-4c7f-bf21-c9bd4fd6147a Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
… in replay handlers Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/d91a8228-5499-4c7f-bf21-c9bd4fd6147a Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1790 +/- ##
=======================================
Coverage 62.37% 62.37%
=======================================
Files 285 285
Lines 8421 8421
Branches 980 980
=======================================
Hits 5253 5253
Misses 2935 2935
Partials 233 233
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@copilot This should target the feature-1.18.0 branch instead of master as it relates to future runtime work and isn't relevant to 1.17. Further, please ensure that any unit or integration tests are similarly gated with the |
Add Dapr.Testcontainers.Xunit project reference to test project and
replace [Fact] with [MinimumDaprRuntimeFact("1.18")] on all 13 timer
origin tests so they are skipped when running against Dapr < 1.18.
Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/b1380761-16ac-42d3-9368-f77c2bd1948d
Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
Done in f288677. All 13 timer origin tests now use The branch is already based on the |
|
@copilot I don't see that you added any integration tests as part of this work. Please do so. |
Add four end-to-end integration tests against a real Dapr sidecar:
1. CreateTimer completes after timer fires
2. WaitForExternalEvent with finite timeout completes when event arrives
3. WaitForExternalEvent with finite timeout returns timed-out when no event
4. WaitForExternalEvent indefinite completes when event arrives
All tests use [MinimumDaprRuntimeFact("1.18")] to skip on older runtimes.
Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/fad6b6d4-43de-40b9-82ec-f4669b8a9b71
Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
…ration tests Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/fad6b6d4-43de-40b9-82ec-f4669b8a9b71 Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
Added in c2bafbf (cleanup in f1b9263). Four integration tests in
|
WhitWaldo
left a comment
There was a problem hiding this comment.
See if you can give this another pass and clean this up some.
| /// Sentinel fireAt value for indefinite external event timers. | ||
| /// Must be exactly 9999-12-31T23:59:59.999999999Z. | ||
| /// </summary> | ||
| internal static readonly Google.Protobuf.WellKnownTypes.Timestamp ExternalEventIndefiniteFireAt = |
There was a problem hiding this comment.
@copilot Not sure I want this in here as I feel it muddies the context type with third party types.
There was a problem hiding this comment.
Moved the ExternalEventIndefiniteFireAt sentinel, SetTimerOrigin, and optional-timer recognition predicates to a new TimerOriginHelpers internal static class in c7c3971. The context class now uses using static TimerOriginHelpers; to keep call sites clean without exposing protobuf types as fields on the context.
| /// <summary> | ||
| /// Creates a durable timer with the specified origin metadata. | ||
| /// </summary> | ||
| internal async Task CreateTimerInternal(DateTime fireAt, IMessage origin, CancellationToken cancellationToken) |
There was a problem hiding this comment.
Do we really need a CreateTimerInternal implementation at both an internal and private visibility or can these be combined?
There was a problem hiding this comment.
Combined into a single private CreateTimerInternal(Timestamp, IMessage, CancellationToken) in c7c3971. All callers (CreateTimer, WaitForExternalEventAsync, CreateActivityRetryTimer, CreateChildWorkflowRetryTimer) now convert to Timestamp at the call site before invoking the single method.
…rge CreateTimerInternal overloads Move ExternalEventIndefiniteFireAt sentinel, SetTimerOrigin, and optional-timer recognition predicates from WorkflowOrchestrationContext to a dedicated TimerOriginHelpers static class. Combine the two CreateTimerInternal overloads into a single private method that accepts Google.Protobuf.WellKnownTypes.Timestamp. Agent-Logs-Url: https://github.com/dapr/dotnet-sdk/sessions/8a7bbcaf-ff06-494a-81cb-3468dba4b31a Co-authored-by: WhitWaldo <2238529+WhitWaldo@users.noreply.github.com>
Done in c7c3971. Extracted protobuf helpers ( |
Description
Implements the timer
originfield onCreateTimerActionandTimerCreatedEvent, making the cause of every timer explicit (explicit create, external event timeout, activity retry delay, child workflow retry delay). Also implements synthetic "optional timers" for indefiniteWaitForExternalEventcalls with a replay-tolerant drop-and-shift algorithm for backwards compatibility with pre-patch histories.This feature targets Dapr runtime 1.18 and all tests are gated with
MinimumDaprRuntimeFact("1.18").Protobuf
TimerOriginCreateTimer,TimerOriginExternalEvent{name},TimerOriginActivityRetry{taskExecutionId},TimerOriginChildWorkflowRetry{instanceId}originoneof added to bothCreateTimerActionandTimerCreatedEventOrigin assignment
CreateTimer()→TimerOriginCreateTimer{}WaitForExternalEvent(name, timeout > 0)→TimerOriginExternalEvent{name}WaitForExternalEvent(name, timeout < 0)→TimerOriginExternalEvent{name}+ sentinelfireAt = 9999-12-31T23:59:59.999999999ZWaitForExternalEvent(name, timeout == 0)→TaskCanceledException, no timer emittedTimerOriginActivityRetry{taskExecutionId}(stable across all retry attempts)TimerOriginChildWorkflowRetry{instanceId}(first-child rule)Replay tolerance
IsOptionalExternalEventTimerAction,IsOptionalExternalEventTimerCreatedEvent), andDropOptionalExternalEventTimerAtdrop-and-shift primitive housed inTimerOriginHelpersstatic classProcessEvents:OnTaskScheduled,OnSubOrchestrationCreated,OnTimerCreated(including asymmetric TimerCreated case where pending is optional but incoming is not)TimerOriginHelpers
TimerOriginHelpersinternal static class inWorker/Internal/keeps protobuf-specific timer origin concerns (sentinel constant,SetTimerOrigin, optional-timer recognition predicates) separate fromWorkflowOrchestrationContext, avoiding muddying the context type with third-party typesRetryInterceptor
retryTimerFactorydelegate so callers can supply origin-aware timer creation without changing the public APICreateTimerInternal
CreateTimerInternal(Timestamp, IMessage, CancellationToken)method handles all timer creation — callers (CreateTimer,WaitForExternalEventAsync,CreateActivityRetryTimer,CreateChildWorkflowRetryTimer) convert toTimestampat the call siteTest gating
Dapr.Testcontainers.Xunitproject reference toDapr.Workflow.TestTimerOriginTestsuse[MinimumDaprRuntimeFact("1.18")]to skip when running against Dapr runtime < 1.18Integration tests
TimerOriginIntegrationTestsinDapr.IntegrationTest.Workflowwith four end-to-end tests against a real Dapr sidecar, all gated with[MinimumDaprRuntimeFact("1.18")]:CreateTimer_ShouldCompleteWorkflow_AfterTimerFires— verifiesCreateTimerwithTimerOriginCreateTimercompletes end-to-endWaitForExternalEvent_FiniteTimeout_ShouldComplete_WhenEventArrivesBeforeTimeout— event arrives before the finite-timeout timer firesWaitForExternalEvent_FiniteTimeout_ShouldTimeout_WhenNoEventArrives— no event raised, the finite-timeout timer fires and the workflow catchesTaskCanceledExceptionWaitForExternalEvent_Indefinite_ShouldComplete_WhenEventArrives— indefinite wait (synthetic optional timer with sentinel fireAt) completes when the event is raisedIssue reference
#1787
Checklist