-
Notifications
You must be signed in to change notification settings - Fork 533
Read Consistency Strategy: Adds hub region header for LastCommittedWriteRegion strategy. #5815
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
b5e72b2
57e2e02
cddf8f1
b009087
a24b765
8db9406
5bcbbaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -529,6 +529,10 @@ private Task ValidateAndSetReadConsistencyStrategyAsync(RequestMessage requestMe | |
| requestMessage.Headers.Set( | ||
| HttpConstants.HttpHeaders.ReadConsistencyStrategy, | ||
| readConsistencyStrategy.Value.ToString()); | ||
|
|
||
| requestMessage.Headers.Set( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hubregion processing should only be triggered by a single ReadConsistencyStartegy enum value - like LastCommittedWriteRegion or similar. Not for all ReadConsistencyStartegies for sure!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hub region header is now only set when ReadConsistencyStrategy == LastCommittedWriteRegion AND the operation is a read (OperationTypeExtensions.IsReadOperation()). |
||
| HttpConstants.HttpHeaders.ShouldProcessOnlyInHubRegion, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Recommendation — Hub header applies to all operation types (including writes) The new hub region header is set unconditionally whenever The sibling method ValidationHelpers.IsValidConsistencyLevelOverwrite(
operationType: requestMessage.OperationType,
resourceType: requestMessage.ResourceType)This method performs no such check. Since Concrete scenario: A customer sets Suggestion: Either:
Note: This is also a cross-SDK divergence — the Java SDK's
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hub region header is now only set when ReadConsistencyStrategy == LastCommittedWriteRegion AND the operation is a read (OperationTypeExtensions.IsReadOperation()). |
||
| bool.TrueString); | ||
| } | ||
|
|
||
| return Task.CompletedTask; | ||
|
|
||
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.
can this get set even for write operations? If so, it might create a side effect.
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.
Hub region header is now only set when ReadConsistencyStrategy == LastCommittedWriteRegion AND the operation is a read (OperationTypeExtensions.IsReadOperation()).
Write operations will never get this header regardless of the strategy configured.