-
Notifications
You must be signed in to change notification settings - Fork 2k
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
API for threshold based retries #35166
API for threshold based retries #35166
Conversation
…ide regions to skip from the retry execution
...s/azure-cosmos/src/main/java/com/azure/cosmos/availabilitystrategy/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
...s/azure-cosmos/src/main/java/com/azure/cosmos/availabilitystrategy/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
...s/azure-cosmos/src/main/java/com/azure/cosmos/availabilitystrategy/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/azure/cosmos/implementation/directconnectivity/ReplicatedResourceClient.java
Outdated
Show resolved
Hide resolved
Refactoring
…based-retries-with-excludelists # Conflicts: # sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/ImplementationBridgeHelpers.java # sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosQueryRequestOptions.java
API change check APIView has identified API level changes in this PR and created following API reviews. |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/AvailabilityStrategy.java
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/BridgeInternal.java
Outdated
Show resolved
Hide resolved
...cosmos/src/main/java/com/azure/cosmos/CosmosEndToEndOperationLatencyPolicyConfigBuilder.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/ThresholdBasedAvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
...ure-cosmos/src/main/java/com/azure/cosmos/implementation/apachecommons/lang/StringUtils.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/azure/cosmos/implementation/directconnectivity/ReplicatedResourceClient.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/azure/cosmos/implementation/directconnectivity/ReplicatedResourceClient.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosQueryRequestOptions.java
Outdated
Show resolved
Hide resolved
...s/azure-cosmos-tests/src/test/java/com/azure/cosmos/EndToEndTimeOutWithAvailabilityTest.java
Show resolved
Hide resolved
// Now try the same request with West US 2 excluded | ||
options.setExcludeRegions(ImmutableList.of("West US 2")); | ||
cosmosItemResponseMono = | ||
createdContainer.readItem(itemToRead.getId(), new PartitionKey(itemToRead.getMypk()), options, EndToEndTimeOutValidationTests.TestObject.class); |
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.
should we verify the contactedRegion?
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/ThresholdBasedAvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
.../azure-cosmos/src/main/java/com/azure/cosmos/CosmosEndToEndOperationLatencyPolicyConfig.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/ThresholdBasedAvailabilityStrategy.java
Show resolved
Hide resolved
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, thanks @mbhaskar
.../azure-cosmos/src/main/java/com/azure/cosmos/CosmosEndToEndOperationLatencyPolicyConfig.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/ThresholdBasedAvailabilityStrategy.java
Show resolved
Hide resolved
Dismissing the review as the feedback comments have been incorporated and resolved.
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/AvailabilityStrategy.java
Outdated
Show resolved
Hide resolved
...cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/GlobalEndpointManager.java
Outdated
Show resolved
Hide resolved
...c/main/java/com/azure/cosmos/implementation/directconnectivity/ReplicatedResourceClient.java
Show resolved
Hide resolved
...c/main/java/com/azure/cosmos/implementation/directconnectivity/ReplicatedResourceClient.java
Outdated
Show resolved
Hide resolved
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/models/CosmosQueryRequestOptions.java
Show resolved
Hide resolved
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.
Thanks - LGTM
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/check-enforcer override |
Description
As end to end policy may effect the availability due to its faster timeouts, we need strategy to improve availability when endto end timeout is specified.
This PR introduces threshold based retry execution of the requests to improve availability when end to end timeout is specified.
API :
This needs
List of regions to be excluded for the request/retries. Example "East US" or "East US, West US" These regions will be excluded from the preferred regions list when executing multi region retries.
The idea here is to provide an option to the user to give a list of regions that they dont want a request to be retried on. This list ideally has to be a sublist of the preferred regions.
CosmosItemRequestOptions#setExcludeRegions
exclude regions can be used in two ways.
Example scneario:
Preferred regions: regionA, regionB, regionC
This can help the scenarios where the user wants to specifically try the request on a particular region.
Note that this works only when EndToEndTimeoutPolicy is set.
This PR introduces a strategy called ThresholdBasedAvailabilityStrategy
ThresholdBasedAvailabilityStrategy contains the following parameters
Threshold is the duration in ms before which if the original request doesnt respond, we issue a request to next region from the list of effective regions.
Effective regions are computed as below. And retries happen only on available effective regions
Example:
Prefrerred regions: "East US 1", "East US 2", "Central US", "West US 2"
Exclude list: "East US 2"
number of Regions = 2
Effective regions = "East US 1", "Central US"
How to use this ?
Enabling it on the client
This can be enabled or disabled per operation
Per operation option always overrides the client option.
How does this work?
Initial execution on the requested region starts at t0;
If no response has been received at t0 + threshold milliseconds, it starts another request on the first region from effectiveRegionList
if no response has been received at t0 + threshold+ threshold_step milliseconds, start another request on the region from the next region from effectiveRegionList
This continues only until the number that is set by the user in the options on the number of regions to request to.
Future:
There is a possibility of extending this to configure individual timeout and availability strategy for different kind of operations like point and non point operations
Possible API on the client
All SDK Contribution checklist:
General Guidelines and Best Practices
Testing Guidelines