Adding new logic for extended sessions to enable the DTS use-case#1333
Adding new logic for extended sessions to enable the DTS use-case#1333
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an explicit “end session” hook to the extended sessions (sticky sessions) flow so providers (e.g., DTS) can release a session/work item when the extended session finishes.
Changes:
- Introduces
IOrchestrationSession.EndSessionAsync()as a new session-termination callback. - Updates orchestration and entity dispatchers to call
EndSessionAsync()when an extended session ends (after releasing the in-proc concurrency lock). - Adds a no-op
EndSessionAsync()implementation for the Azure StorageOrchestrationSession.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/DurableTask.Core/TaskOrchestrationDispatcher.cs | Calls EndSessionAsync() when releasing an extended session; adjusts rewind handling logic. |
| src/DurableTask.Core/TaskEntityDispatcher.cs | Calls EndSessionAsync() when releasing an extended session in the entity dispatcher. |
| src/DurableTask.Core/IOrchestrationSession.cs | Adds the new EndSessionAsync() method to the public session interface. |
| src/DurableTask.AzureStorage/Messaging/OrchestrationSession.cs | Implements EndSessionAsync() as a no-op for the Azure Storage provider. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <summary> | ||
| /// Interface allowing providers to implement extended sessions (aka "sticky sessions"). | ||
| /// </summary> | ||
| public interface IOrchestrationSession |
There was a problem hiding this comment.
changing a public interface is a breaking change and, as such, requires a major version change.
This could break a lot of code all over the place.
There was a problem hiding this comment.
Technically yes, but only Azure Storage implements this interface and it is updated in this same PR so in practice it won't be one
There was a problem hiding this comment.
That being said, I will discuss the nature of the "breaking change" and whether we do want this to be a major version update with the rest of the team. The version change will appear in a separate PR either way
cc @torosent
| // Setting this to true here will end an extended session if it is in progress. | ||
| // We don't want to save the state across executions, since we essentially manually modify | ||
| // the orchestration history here and so that stored by the extended session is incorrect. | ||
| isRewinding = true; |
There was a problem hiding this comment.
why is this assignment no longer needed?
There was a problem hiding this comment.
This was leftover from a previous PR where I added rewind support. I realized that I had moved where this was set to here but forgot to remove this redundant setting later on.
This PR introduces new logic in the extended sessions flow that is necessary for the DTS use-case. In particular, it introduces a new
EndSessionAsynccall which will allow DTS to release the work item once the extended session ends.