-
Notifications
You must be signed in to change notification settings - Fork 523
Per Partition Automatic Failover: Adds Hub Region Processing Only While Routing Requests Failed with 404/1002 for single master accounts. #5447
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: master
Are you sure you want to change the base?
Changes from 1 commit
7e9b945
7786e45
d85ed0a
163be31
e523341
822b8e6
1ad3fef
1f63898
2d90cb1
b1fa2b2
35e0eeb
d51b105
f622ebd
a15e97a
961288f
b8facfd
1fbd6c0
7668f2a
692484c
9dd4ec0
66d481b
5b541e1
7936ada
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 |
|---|---|---|
|
|
@@ -22,7 +22,8 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy | |
| { | ||
| private const int RetryIntervalInMS = 1000; // Once we detect failover wait for 1 second before retrying request. | ||
| private const int MaxRetryCount = 120; | ||
| private const int MaxServiceUnavailableRetryCount = 1; | ||
| private const int MaxServiceUnavailableRetryCount = 1; | ||
| private const string HubRegionHeader = "x-ms-cosmos-hub-region-processing-only"; | ||
|
|
||
| private readonly IDocumentClientRetryPolicy throttlingRetry; | ||
| private readonly GlobalEndpointManager globalEndpointManager; | ||
|
|
@@ -38,7 +39,8 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy | |
| private bool isMultiMasterWriteRequest; | ||
| private Uri locationEndpoint; | ||
| private RetryContext retryContext; | ||
| private DocumentServiceRequest documentServiceRequest; | ||
| private DocumentServiceRequest documentServiceRequest; | ||
| private bool addHubRegionProcessingOnlyHeader; | ||
|
||
|
|
||
| public ClientRetryPolicy( | ||
| GlobalEndpointManager globalEndpointManager, | ||
|
|
@@ -222,6 +224,12 @@ public void OnBeforeSendRequest(DocumentServiceRequest request) | |
| // set location-based routing directive based on request retry context | ||
| request.RequestContext.RouteToLocation(this.retryContext.RetryLocationIndex, this.retryContext.RetryRequestOnPreferredLocations); | ||
| } | ||
| } | ||
| // If previous attempt failed with 404/1002, add the hub-region-processing-only header | ||
| if (this.addHubRegionProcessingOnlyHeader) | ||
| { | ||
| request.Headers[HubRegionHeader] = bool.TrueString; | ||
| this.addHubRegionProcessingOnlyHeader = false; // reset after applying | ||
|
||
| } | ||
|
|
||
| // Resolve the endpoint for the request and pin the resolution to the resolved endpoint | ||
|
|
@@ -322,7 +330,8 @@ private async Task<ShouldRetryResult> ShouldRetryInternalAsync( | |
|
|
||
| if (statusCode == HttpStatusCode.NotFound | ||
| && subStatusCode == SubStatusCodes.ReadSessionNotAvailable) | ||
|
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. also double check: does this change also targeted for MM as well? For MM, writes can happen in any region, also enable this for MM might cause regression
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. confirmed this with backend team and this change is not intended to be used in multi-master. |
||
| { | ||
| { | ||
| this.addHubRegionProcessingOnlyHeader = true; | ||
aavasthy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return this.ShouldRetryOnSessionNotAvailable(this.documentServiceRequest); | ||
| } | ||
|
|
||
|
|
@@ -338,7 +347,7 @@ private async Task<ShouldRetryResult> ShouldRetryInternalAsync( | |
| || (statusCode == HttpStatusCode.Gone && subStatusCode == SubStatusCodes.LeaseNotFound)) | ||
| { | ||
| return this.ShouldRetryOnUnavailableEndpointStatusCodes(); | ||
| } | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
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.
Do we have any central place to host these types of headers in the code base?
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.
yes we are actually using the central file HttpConstants.cs. I removed this line as part of code clean up.