Rewind fails to rewind activities that were called with retry
- Dominant language
- C#
- Stars
- 1.7k
- Forks
- 335
- Avg merge
- 2d 23h
- Merged PRs (30d)
- 6
Description
When calling an activity with retry (via `ScheduleTask` via `CallActivityWithRetryAsync`, from durable task extension), the `RetryInterceptor` retries the activities if they fail. For each retry, a `TimerCreated` (and consequent `TimerFired`) events are added to the history of the orchestration.
(And due to the following:
```c#
if (isLastRetry)
{
// Earlier versions of this retry interceptor had a bug that scheduled an extra delay timer.
// It's unfortunately not possible to remove the extra timer since that would potentially
// break the history replay for existing orchestrations. Instead, we do the next best thing
// and schedule a timer that fires immediately instead of waiting for a full delay interval.
await this.context.CreateTimer(this.context.CurrentUtcDateTime, "Dummy timer for back-compat");
break;
}
```
after the last attempt, another `TimerEvent` and `TimerFired` event are added).
When `AzureTableTrackingStore.RewindHistoryAsync` is called to rewind the orchestration, only `TaskFailed` and `SubOrchestrationInstanceFailed` (and their corresponding `TaskScheduled` and `SubOrchestrationInstanceCreated`) get their `EventType` reset to `GenericEvent`. So when the orchestration restarts, it encounters `TimerCreated` and `TimerFired` events that it did not expect, and causes the following error:
```
Non-Deterministic workflow detected: A previous execution of this orchestration scheduled a
timer task with sequence number 1 but the current replay execution hasn't (yet?) scheduled this
task. Was a change made to the orchestrator code after this instance had already startedrunning?
```
I think to fix this, the rewind algorithm should take the timer events into account, and also overwrite their `EventType` to `GenericEvent`. I've tested this by modifying the table storage entries before rewinding and that works. I can imagine that the fix is to find all the `TimerCreated` events that have an `EventId` higher than the `TaskScheduled` that is being reset. The corresponding `TimerFired` events can be found using the `TimerId` property.
I don't mind implementing the fix for this, but I would like to know if this is the best approach. I can imagine that this change can inadvertently reset some timers it should not touch. But as the Rewind algorithm just resets all the `TaskFailed` events, resetting the timer events after those events might just work fine.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.