Skip to content

Implemented timers created for activity and workflow retries#80

Merged
JoshVanL merged 4 commits intodapr:mainfrom
acroca:retry-timer-origin
Apr 8, 2026
Merged

Implemented timers created for activity and workflow retries#80
JoshVanL merged 4 commits intodapr:mainfrom
acroca:retry-timer-origin

Conversation

@acroca
Copy link
Copy Markdown
Member

@acroca acroca commented Apr 8, 2026

When a timer is created to perform an activity or a workflow retry, we set a specific timer origin to the timer.

In the case of workflow retry timers, the timer origin points always to the instance ID of the first attempt

Signed-off-by: Albert Callarisa <albert@diagrid.io>
@acroca acroca requested a review from a team as a code owner April 8, 2026 10:02
if options.retryPolicy != nil {
// Compute the first child's instance ID for the timer origin.
// Each retry still gets its own instance ID from the applier.
firstInstanceID := options.instanceID
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a pointer instead to check for nil or not worth/not able to do?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options.instanceID is already a string, I wouldn't change that, at least not in this PR

Signed-off-by: Albert Callarisa <albert@diagrid.io>
Copy link
Copy Markdown

Copilot AI left a 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 extends timer metadata so timers created as part of activity retries and child workflow retries carry a specific “origin” payload, and for child-workflow retry timers the origin consistently points to the first attempt’s instance ID.

Changes:

  • Add new timer-origin variants for ActivityRetry and ChildWorkflowRetry and propagate them from CreateTimerAction into TimerCreatedEvent.
  • Update orchestration retry logic to set the appropriate timer origin when creating retry-delay timers.
  • Add/extend tests covering the new timer-origin behavior (including the “points to first child attempt” rule) and centralize child instance ID generation via a helper.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
task/orchestrator.go Sets timer origins for retry-delay timers; refactors timer creation to allow origin mutation.
backend/runtimestate/applier.go Maps CreateTimerAction.Origin to TimerCreatedEvent.Origin; uses helper for deterministic child instance IDs.
api/helpers/task.go Introduces helper to generate deterministic child workflow instance IDs.
tests/runtimestate_test.go Adds unit-style tests validating retry timer origins and first-attempt instance ID semantics.
tests/orchestrations_test.go Adds end-to-end test for child workflow retries with auto-generated instance IDs.
api/protos/orchestrator_actions.pb.go Generated updates to support new timer origin oneof variants in actions.
api/protos/history_events.pb.go Generated updates to support new timer origin oneof variants in history events.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 4 to +16
"reflect"
"runtime"
"strconv"
"strings"
)

// GenerateChildWorkflowInstanceID generates a deterministic instance ID for a
// child workflow based on the parent instance ID and the action's sequence number.
func GenerateChildWorkflowInstanceID(parentInstanceID string, actionID int32) string {
// Adding 0x10000 guarantees at least 5 hex digits (e.g. 0 → "10000"),
// so taking the last 4 gives a zero-padded result (e.g. "0000").
hex := strconv.FormatInt(0x10000+int64(actionID), 16)
hex = hex[len(hex)-4:]
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GenerateChildWorkflowInstanceID truncates the hex representation to the last 4 digits (via hex = hex[len(hex)-4:]). For action IDs >= 0x10000 this wraps/collides (e.g., 0 and 65536 both become "0000"), which can produce duplicate child workflow instance IDs and break de-duplication/correlation. Preserve the full hex value while still left-padding to at least 4 characters (e.g., %04x formatting without truncation, or manual zero-padding when len(hex) < 4).

Suggested change
"reflect"
"runtime"
"strconv"
"strings"
)
// GenerateChildWorkflowInstanceID generates a deterministic instance ID for a
// child workflow based on the parent instance ID and the action's sequence number.
func GenerateChildWorkflowInstanceID(parentInstanceID string, actionID int32) string {
// Adding 0x10000 guarantees at least 5 hex digits (e.g. 0 → "10000"),
// so taking the last 4 gives a zero-padded result (e.g. "0000").
hex := strconv.FormatInt(0x10000+int64(actionID), 16)
hex = hex[len(hex)-4:]
"fmt"
"reflect"
"runtime"
"strings"
)
// GenerateChildWorkflowInstanceID generates a deterministic instance ID for a
// child workflow based on the parent instance ID and the action's sequence number.
func GenerateChildWorkflowInstanceID(parentInstanceID string, actionID int32) string {
// Format with a minimum width of 4 hex digits while preserving the full value
// for action IDs that require more than 4 digits.
hex := fmt.Sprintf("%04x", actionID)

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @JoshVanL :D

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:)

acroca added 2 commits April 8, 2026 16:34
Signed-off-by: Albert Callarisa <albert@diagrid.io>
Signed-off-by: Albert Callarisa <albert@diagrid.io>
@JoshVanL JoshVanL merged commit 7e0554e into dapr:main Apr 8, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants