-
Notifications
You must be signed in to change notification settings - Fork 525
HttpTimeoutPolicy Improvements Phase 3: Fixes the TimeoutPolicies to Have Segregated Timeouts for Point-Reads and Non-Point-Reads with Uniform Timeouts #5497
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
Conversation
…he configurable timeouts.
Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForPartitionFailover.cs
Show resolved
Hide resolved
…ient http policy.
Microsoft.Azure.Cosmos/src/HttpClient/HttpTimeoutPolicyForThinClient.cs
Outdated
Show resolved
Hide resolved
| (TimeSpan.FromSeconds(1), TimeSpan.Zero), | ||
| (TimeSpan.FromSeconds(6), TimeSpan.Zero), | ||
| (TimeSpan.FromSeconds(6), TimeSpan.Zero), | ||
| (TimeSpan.FromSeconds(10), TimeSpan.Zero), |
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.
Please update PR description with more context (based on our conversation)
kundadebdatta
left a comment
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.
LGTM: Let's wait for the detailed PR description before merging this into master.
FabianMeiswinkel
left a comment
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.
LGTM
|
#auto-merge |
Pull Request Template
Description
At a high-level, this PR aims to unify the timeouts across PPAF and thin client http retry policies for read only requests. Furthermore, HttpTimeoutPolicies for PPAF and Thinclient will have segregated timeouts for Point-Reads and Non-Point-Reads. With this change, we will have the ability to fine-tune/optimize the timeouts for the respective operations. In this context, requests are allowed to retry 3 times with 6s, 6s and 10s timeouts respectively for each retry. We chose these timeouts based on the default backend response timeouts and it will help requests being timeout with aggressive timeouts.
As next step, we have planned for DR drill to identify the appropriate timeouts. Depending on the findings from the drill, we may go towards the direction of configurable timeout for first retry.
Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #5496