-
Notifications
You must be signed in to change notification settings - Fork 298
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
Optimize Continue As New And Solve Missed Raised Events #251
Conversation
Note: this fixes Azure/azure-functions-durable-extension#67 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm super-excited to have this change! Relatively minor CR feedback.
Test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Outdated
Show resolved
Hide resolved
Test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.Core/Command/OrchestrationCompleteOrchestratorAction.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.Core/Command/OrchestrationCompleteOrchestratorAction.cs
Outdated
Show resolved
Hide resolved
test/DurableTask.ServiceBus.Tests/ServiceBusOrchestrationServiceTests.cs
Outdated
Show resolved
Hide resolved
Test/DurableTask.AzureStorage.Tests/AzureStorageScenarioTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for making these changes.
Need to add one more change for the azurestorageprovider which is to also commit the old runtime state if it is instance store backed |
src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
Outdated
Show resolved
Hide resolved
@@ -13,8 +13,16 @@ | |||
|
|||
namespace DurableTask.Core.Command | |||
{ | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra newline
src/DurableTask.Core/Command/OrchestrationCompleteOrchestratorAction.cs
Outdated
Show resolved
Hide resolved
continuedAsNew = false; | ||
continuedAsNewMessage = null; | ||
|
||
TraceHelper.TraceInstance( | ||
TraceEventType.Verbose, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indenting
src/DurableTask.AzureStorage/Tracking/InstanceStoreBackedTrackingStore.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Optimize Continue As New And Solve Missed Events * make skip events property driven * Commit old state in case of an instance store backed tracking
This PR aims to optimize ContinueAsNew so that an orchestration continues in memory in the dispatcher if it can instead of enqueueing a new task message.
With this Events Raised After a continueAsNew has been executed will also be collected and carried over to the next iteration of the orchestration. This a feature that can be opted out of by specifying SkipEventsOnContinuation as true in the OrchestrationService.