Skip to content

Commit

Permalink
Convert timer target to UTC (#1138)
Browse files Browse the repository at this point in the history
Convert timer target time to UTC

---------

Co-authored-by: David Justo <[email protected]>
  • Loading branch information
AnatoliB and davidmrdavid authored Jul 23, 2024
1 parent b04e395 commit b01ef27
Show file tree
Hide file tree
Showing 4 changed files with 115 additions and 3 deletions.
13 changes: 13 additions & 0 deletions src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1106,6 +1106,17 @@ public async Task CompleteTaskOrchestrationWorkItemAsync(
TaskMessage continuedAsNewMessage,
OrchestrationState orchestrationState)
{
// for backwards compatibility, we transform timer timestamps to UTC prior to persisting in Azure Storage.
// see: https://github.com/Azure/durabletask/pull/1138
foreach (var orchestratorMessage in orchestratorMessages)
{
Utils.ConvertDateTimeInHistoryEventsToUTC(orchestratorMessage.Event);
}
foreach (var timerMessage in timerMessages)
{
Utils.ConvertDateTimeInHistoryEventsToUTC(timerMessage.Event);
}

OrchestrationSession session;
if (!this.orchestrationSessionManager.TryGetExistingSession(workItem.InstanceId, out session))
{
Expand Down Expand Up @@ -1687,6 +1698,8 @@ public async Task CreateTaskOrchestrationAsync(TaskMessage creationMessage, Orch
throw new ArgumentException($"Only {nameof(EventType.ExecutionStarted)} messages are supported.", nameof(creationMessage));
}

Utils.ConvertDateTimeInHistoryEventsToUTC(creationMessage.Event);

// Client operations will auto-create the task hub if it doesn't already exist.
await this.EnsureTaskHubAsync();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -846,6 +846,9 @@ public override Task StartAsync(CancellationToken cancellationToken = default)
bool isFinalEvent = i == newEvents.Count - 1;

HistoryEvent historyEvent = newEvents[i];
// For backwards compatibility, we convert timer timestamps to UTC prior to persisting to Azure Storage
// see: https://github.com/Azure/durabletask/pull/1138
Utils.ConvertDateTimeInHistoryEventsToUTC(historyEvent);
var historyEntity = TableEntityConverter.Serialize(historyEvent);
historyEntity.PartitionKey = sanitizedInstanceId;

Expand Down
31 changes: 31 additions & 0 deletions src/DurableTask.AzureStorage/Utils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -272,5 +272,36 @@ public static object DeserializeFromJson(JsonSerializer serializer, string jsonS
}
return obj;
}

public static void ConvertDateTimeInHistoryEventsToUTC(HistoryEvent historyEvent)
{
switch (historyEvent.EventType)
{
case EventType.ExecutionStarted:
var executionStartedEvent = (ExecutionStartedEvent)historyEvent;
if (executionStartedEvent.ScheduledStartTime.HasValue &&
executionStartedEvent.ScheduledStartTime.Value.Kind != DateTimeKind.Utc)
{
executionStartedEvent.ScheduledStartTime = executionStartedEvent.ScheduledStartTime.Value.ToUniversalTime();
}
break;

case EventType.TimerCreated:
var timerCreatedEvent = (TimerCreatedEvent)historyEvent;
if (timerCreatedEvent.FireAt.Kind != DateTimeKind.Utc)
{
timerCreatedEvent.FireAt = timerCreatedEvent.FireAt.ToUniversalTime();
}
break;

case EventType.TimerFired:
var timerFiredEvent = (TimerFiredEvent)historyEvent;
if (timerFiredEvent.FireAt.Kind != DateTimeKind.Utc)
{
timerFiredEvent.FireAt = timerFiredEvent.FireAt.ToUniversalTime();
}
break;
}
}
}
}
71 changes: 68 additions & 3 deletions test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1285,6 +1285,71 @@ public async Task TimerExpiration(bool enableExtendedSessions)
}
}

[DataTestMethod]
[DataRow(true)]
[DataRow(false)]
public async Task TimerDelay(bool useUtc)
{
using (TestOrchestrationHost host = TestHelpers.GetTestOrchestrationHost(false))
{
await host.StartAsync();
// by convention, DateTime objects are expected to be in UTC, but previous version of DTFx.AzureStorage
// performed a implicit conversions to UTC when different timezones where used. This test ensures
// that behavior is backwards compatible, despite not being recommended.
var startTime = useUtc ? DateTime.UtcNow : DateTime.Now;
var delay = TimeSpan.FromSeconds(5);
var fireAt = startTime.Add(delay);
var client = await host.StartOrchestrationAsync(typeof(Orchestrations.DelayedCurrentTimeInline), fireAt);

var status = await client.WaitForCompletionAsync(TimeSpan.FromSeconds(30));
Assert.AreEqual(OrchestrationStatus.Completed, status?.OrchestrationStatus);

var actualDelay = DateTime.UtcNow - startTime.ToUniversalTime();
Assert.IsTrue(
actualDelay >= delay && actualDelay < delay + TimeSpan.FromSeconds(10),
$"Expected delay: {delay}, ActualDelay: {actualDelay}");

await host.StopAsync();
}
}

[DataTestMethod]
[DataRow(false)]
[DataRow(true)]
public async Task OrchestratorStartAtAcceptsAllDateTimeKinds(bool useUtc)
{
using (TestOrchestrationHost host = TestHelpers.GetTestOrchestrationHost(false))
{
await host.StartAsync();
// by convention, DateTime objects are expected to be in UTC, but previous version of DTFx.AzureStorage
// performed a implicit conversions to UTC when different timezones where used. This test ensures
// that behavior is backwards compatible, despite not being recommended.

// set up orchestrator start time
var currentTime = DateTime.Now;
var delay = TimeSpan.FromSeconds(5);
var startAt = currentTime.Add(delay);

if (useUtc)
{
startAt = startAt.ToUniversalTime();
}


var client = await host.StartOrchestrationAsync(typeof(Orchestrations.CurrentTimeInline), input: string.Empty, startAt: startAt);

var status = await client.WaitForCompletionAsync(TimeSpan.FromSeconds(30));
Assert.AreEqual(OrchestrationStatus.Completed, status?.OrchestrationStatus);

var orchestratorState = await client.GetStateAsync(client.InstanceId);
var actualScheduledStartTime = status.ScheduledStartTime;

// internal representation of DateTime is always UTC
var expectedScheduledStartTime = startAt.ToUniversalTime();
Assert.AreEqual(expectedScheduledStartTime, actualScheduledStartTime);
await host.StopAsync();
}
}
/// <summary>
/// End-to-end test which validates that orchestrations run concurrently of each other (up to 100 by default).
/// </summary>
Expand Down Expand Up @@ -3304,11 +3369,11 @@ public override Task<DateTime> RunTask(OrchestrationContext context, string inpu
}
}

internal class DelayedCurrentTimeInline : TaskOrchestration<DateTime, string>
internal class DelayedCurrentTimeInline : TaskOrchestration<DateTime, DateTime>
{
public override async Task<DateTime> RunTask(OrchestrationContext context, string input)
public override async Task<DateTime> RunTask(OrchestrationContext context, DateTime fireAt)
{
await context.CreateTimer<bool>(context.CurrentUtcDateTime.Add(TimeSpan.FromSeconds(3)), true);
await context.CreateTimer<bool>(fireAt, true);
return context.CurrentUtcDateTime;
}
}
Expand Down

0 comments on commit b01ef27

Please sign in to comment.