Introduce access to session state - #1503
lailabougria wants to merge 7 commits into
Conversation
| using System.Threading.Tasks; | ||
| using Azure.Messaging.ServiceBus; | ||
|
|
||
| interface ISessionStateStore |
There was a problem hiding this comment.
Do we need this internal interface? I can only spot a single implementation of it
| current.UserState = new UserSessionState | ||
| { | ||
| ContentType = "application/json", | ||
| Type = typeof(T).FullName, |
There was a problem hiding this comment.
Do we need to store this? What's the expected usage?
There was a problem hiding this comment.
I was thinking that this could be emited in observability
|
|
||
| current.UserState = new UserSessionState | ||
| { | ||
| ContentType = "application/json", |
There was a problem hiding this comment.
If the Data is defined as JsonElement, can ContentType be anything other than json?
| /// <summary> | ||
| /// Persists pending session-state changes, if any. Called by the session pump right before the message is completed. | ||
| /// </summary> | ||
| public async Task Flush(CancellationToken cancellationToken = default) |
There was a problem hiding this comment.
Do we want have the Save-Flush model enforced in our API?
It seems to be appropriate if the underlying SetSessionStateAsync operations are atomic with the message settlement but currently in our transport they are atomic only when using SendsAtomicWithReceive (default) transaction mode. When using ReceiveOnly, there is no TransactionScope surrounding the ASB API calls and as a result the SetSessionStateAsync is committed separately from Complete/Abandon.
As a result, we might get into some interesting gotachas:
- With outbox disabled, the session might be updated but the message fails to complete and comes back
- With outbox enabled, the message might be marked as processed in the outbox but later the
Flushmight fail and the session state update associated with the message handling is lost
Should we limit the session-state API to the SendsAtomicWithReceive mode (at least in the first release) to avoid these anomalies?
SzymonPobiega
left a comment
SzymonPobiega
left a comment
There was a problem hiding this comment.
I added some comments. Some are purely esthetic but the comment on the Flash method is more interesting -- documents transactional anomalies that we are exposed to.
2 participants
No description provided.