-
Notifications
You must be signed in to change notification settings - Fork 525
PPAF: Fixes issue where setting RequestTimeout to 0 second will cause PPAF dynamic enablement to break
#5427
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
Merged
microsoft-github-policy-service
merged 5 commits into
master
from
users/nalutripician/ppaf_with_0s_requestTimeout-fix
Oct 1, 2025
Merged
PPAF: Fixes issue where setting RequestTimeout to 0 second will cause PPAF dynamic enablement to break
#5427
microsoft-github-policy-service
merged 5 commits into
master
from
users/nalutripician/ppaf_with_0s_requestTimeout-fix
Oct 1, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kundadebdatta
previously approved these changes
Oct 1, 2025
Member
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
…ttps://github.com/Azure/azure-cosmos-dotnet-v3 into users/nalutripician/ppaf_with_0s_requestTimeout-fix
RequestTimeout to 0 second will cause PPAF dynamic enablement to break
kundadebdatta
previously approved these changes
Oct 1, 2025
Member
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
kundadebdatta
approved these changes
Oct 1, 2025
FabianMeiswinkel
approved these changes
Oct 1, 2025
Member
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-Thx
NaluTripician
added a commit
that referenced
this pull request
Oct 7, 2025
…ause PPAF dynamic enablement to break (#5427) # Pull Request Template ## Description This pull request improves the handling of invalid request timeout values in the Cosmos client and adds a new integration test to verify the behavior. The main focus is ensuring that when a request timeout of 0ms is set, the client falls back to a safe default for hedging thresholds, preventing potential issues with failover strategies. **Partition-level failover and hedging logic improvements:** * Updated `InitializePartitionLevelFailoverWithDefaultHedging` in `DocumentClient.cs` to check if `RequestTimeout` is set to 0ms, and if so, fallback to `DefaultHedgingThresholdInMilliseconds` and log a warning. This prevents invalid timeout values from causing issues with availability strategies. **Testing and validation:** * Added a new test method `ClinetOverrides0msRequestTimeoutValueForPPAF` in `CosmosItemIntegrationTests.cs` to verify that when the client is configured with a 0ms request timeout, the hedging threshold is not set to 0, ensuring the fallback logic works as intended. The test also intercepts gateway responses to simulate specific conditions. ## Type of change Please delete options that are not relevant. - [] Bug fix (non-breaking change which fixes an issue) ## Closing issues To automatically close an issue: closes #5430 --------- Co-authored-by: Debdatta Kunda <[email protected]>
NaluTripician
added a commit
that referenced
this pull request
Oct 7, 2025
…ause PPAF dynamic enablement to break (#5427) This pull request improves the handling of invalid request timeout values in the Cosmos client and adds a new integration test to verify the behavior. The main focus is ensuring that when a request timeout of 0ms is set, the client falls back to a safe default for hedging thresholds, preventing potential issues with failover strategies. **Partition-level failover and hedging logic improvements:** * Updated `InitializePartitionLevelFailoverWithDefaultHedging` in `DocumentClient.cs` to check if `RequestTimeout` is set to 0ms, and if so, fallback to `DefaultHedgingThresholdInMilliseconds` and log a warning. This prevents invalid timeout values from causing issues with availability strategies. **Testing and validation:** * Added a new test method `ClinetOverrides0msRequestTimeoutValueForPPAF` in `CosmosItemIntegrationTests.cs` to verify that when the client is configured with a 0ms request timeout, the hedging threshold is not set to 0, ensuring the fallback logic works as intended. The test also intercepts gateway responses to simulate specific conditions. Please delete options that are not relevant. - [] Bug fix (non-breaking change which fixes an issue) To automatically close an issue: closes #5430 --------- Co-authored-by: Debdatta Kunda <[email protected]>
NaluTripician
added a commit
that referenced
this pull request
Oct 7, 2025
…ause PPAF dynamic enablement to break (#5427) This pull request improves the handling of invalid request timeout values in the Cosmos client and adds a new integration test to verify the behavior. The main focus is ensuring that when a request timeout of 0ms is set, the client falls back to a safe default for hedging thresholds, preventing potential issues with failover strategies. **Partition-level failover and hedging logic improvements:** * Updated `InitializePartitionLevelFailoverWithDefaultHedging` in `DocumentClient.cs` to check if `RequestTimeout` is set to 0ms, and if so, fallback to `DefaultHedgingThresholdInMilliseconds` and log a warning. This prevents invalid timeout values from causing issues with availability strategies. **Testing and validation:** * Added a new test method `ClinetOverrides0msRequestTimeoutValueForPPAF` in `CosmosItemIntegrationTests.cs` to verify that when the client is configured with a 0ms request timeout, the hedging threshold is not set to 0, ensuring the fallback logic works as intended. The test also intercepts gateway responses to simulate specific conditions. Please delete options that are not relevant. - [] Bug fix (non-breaking change which fixes an issue) To automatically close an issue: closes #5430 --------- Co-authored-by: Debdatta Kunda <[email protected]>
This was referenced Nov 17, 2025
Merged
Merged
Closed
This was referenced Nov 29, 2025
Merged
This was referenced Dec 8, 2025
This was referenced Dec 15, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Description
This pull request improves the handling of invalid request timeout values in the Cosmos client and adds a new integration test to verify the behavior. The main focus is ensuring that when a request timeout of 0ms is set, the client falls back to a safe default for hedging thresholds, preventing potential issues with failover strategies.
Partition-level failover and hedging logic improvements:
InitializePartitionLevelFailoverWithDefaultHedginginDocumentClient.csto check ifRequestTimeoutis set to 0ms, and if so, fallback toDefaultHedgingThresholdInMillisecondsand log a warning. This prevents invalid timeout values from causing issues with availability strategies.Testing and validation:
ClinetOverrides0msRequestTimeoutValueForPPAFinCosmosItemIntegrationTests.csto verify that when the client is configured with a 0ms request timeout, the hedging threshold is not set to 0, ensuring the fallback logic works as intended. The test also intercepts gateway responses to simulate specific conditions.Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #5430