Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert timer target to UTC #1138

Merged
merged 19 commits into from
Jul 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
950f106
Convert timer target time to UTC
AnatoliB Jul 16, 2024
1226501
Validate FireAt value
AnatoliB Jul 16, 2024
7d74739
Add comment to test/DurableTask.AzureStorage.Tests/AzureStorageScenar…
AnatoliB Jul 16, 2024
6723b13
Revert DT.Core changes
AnatoliB Jul 17, 2024
21197a7
Convert time to UTC in AzureTableTrackingStore.UpdateStateAsync
AnatoliB Jul 18, 2024
1825acc
Remove unnecessary enableExtendedSessions permutations from the test
AnatoliB Jul 18, 2024
442ee76
Verify timer delay time
AnatoliB Jul 18, 2024
43bff45
Convert to UTC in CompleteTaskOrchestrationWorkItemAsync
AnatoliB Jul 19, 2024
3496be2
Update src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
AnatoliB Jul 19, 2024
ef3caf6
Update src/DurableTask.AzureStorage/Tracking/AzureTableTrackingStore.cs
AnatoliB Jul 19, 2024
2e0038e
Rename ConvertTimeToUtc to ConvertTimerEventsToUTC
AnatoliB Jul 19, 2024
38c5b7a
Measure the delay in the orchestration code
AnatoliB Jul 22, 2024
0b5b391
Revert "Measure the delay in the orchestration code"
AnatoliB Jul 22, 2024
9755652
Measure the delay in the test code
AnatoliB Jul 22, 2024
6f12d9d
Convert ExecutionStartedEvent.ScheduledStartTime to UTC
AnatoliB Jul 22, 2024
fd77b37
Convert ExecutionStartedEvent.ScheduledStartTime to UTC in CreateTask…
AnatoliB Jul 22, 2024
12ac111
Update test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
AnatoliB Jul 23, 2024
3663824
Fix orchestration name
AnatoliB Jul 23, 2024
7943d02
Merge branch 'main' into anatolib/fix-issue-1127
AnatoliB Jul 23, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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();
AnatoliB marked this conversation as resolved.
Show resolved Hide resolved
}
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();
}
}

AnatoliB marked this conversation as resolved.
Show resolved Hide resolved
[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>
AnatoliB marked this conversation as resolved.
Show resolved Hide resolved
{
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