-
Notifications
You must be signed in to change notification settings - Fork 510
Session Consistency: Adds SessionTokenMismatchRetryPolicy optimization through customer supplied region switch hints #4976
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: msdata/direct
Are you sure you want to change the base?
Changes from 4 commits
4dc4ee2
99241ea
6b2e0b4
f855753
b3580af
b504545
2d80ec9
257f74e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
namespace Microsoft.Azure.Cosmos | ||
{ | ||
using System; | ||
using Microsoft.Azure.Documents; | ||
|
||
/// <summary> | ||
/// Implementation of ISessionRetryOptions interface, do not want clients to subclass. | ||
/// </summary> | ||
public sealed class SessionRetryOptions : ISessionRetryOptions | ||
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. Two possible options
If we environ this surface area expanding richly forward then later makes sense, otherwise feels like overkill thoughts? 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. It also then makes the RetryTime and RetryCunt as implementation details not as public contract. 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. make sense. I will make the changes to take bool as a contract. |
||
{ | ||
/// <summary> | ||
/// Sets the minimum retry time for 404/1002 retries within each region for read and write operations. | ||
/// The minimum value is 100ms - this minimum is enforced to provide a way for the local region to catch-up on replication lag. The default value is 500ms - as a recommendation ensure that this value is higher than the steady-state | ||
/// replication latency between the regions you chose | ||
/// </summary> | ||
public TimeSpan MinInRegionRetryTime { get; set; } = ConfigurationManager.GetMinRetryTimeInLocalRegionWhenRemoteRegionPreferred(); | ||
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. For a single master write region, after one iteration over all replicas we can yield fast right? 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.
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. Yes, this one seems like an optimization. As discussed, I will set up a meeting with Fabien and Abhijeet and you to go over it and we will plan as a follow up PR. |
||
|
||
/// <summary> | ||
/// Sets the maximum number of retries within each region for read and write operations. The minimum value is 1 - the backoff time for the last in-region retry will ensure that the total retry time within the | ||
/// region is at least the min. in-region retry time. | ||
/// </summary> | ||
public int MaxInRegionRetryCount { get; set; } = ConfigurationManager.GetMaxRetriesInLocalRegionWhenRemoteRegionPreferred(); | ||
|
||
|
||
/// <summary> | ||
/// hints which guide SDK-internal retry policies on how early to switch retries to a different region. | ||
/// </summary> | ||
public Boolean RemoteRegionPreferred { get; set; } = false; | ||
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. Can we please expand the text on when the switch will be performed? Like after all the replicas are tried locally? 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. return type 'bool' (Object vs primitive type) 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. fixed. |
||
|
||
|
||
|
||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.