Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 40 additions & 10 deletions Microsoft.Azure.Cosmos/src/ClientRetryPolicy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy
private readonly IDocumentClientRetryPolicy throttlingRetry;
private readonly GlobalEndpointManager globalEndpointManager;
private readonly GlobalPartitionEndpointManager partitionKeyRangeLocationCache;
private readonly bool enableEndpointDiscovery;
private readonly bool enableEndpointDiscovery;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Looks like difference in LF handling is sneaking through.

private readonly bool isThinClientEnabled;
private int failoverRetryCount;

Expand All @@ -51,7 +51,7 @@ internal sealed class ClientRetryPolicy : IDocumentClientRetryPolicy
private bool isDtxRequest;
private Uri locationEndpoint;
private RetryContext retryContext;
private DocumentServiceRequest documentServiceRequest;
private DocumentServiceRequest documentServiceRequest;
#if !INTERNAL
private volatile bool addHubRegionProcessingOnlyHeader;
private CrossRegionAvailabilityContext crossRegionAvailabilityContext;
Expand All @@ -61,7 +61,7 @@ public ClientRetryPolicy(
GlobalEndpointManager globalEndpointManager,
GlobalPartitionEndpointManager partitionKeyRangeLocationCache,
RetryOptions retryOptions,
bool enableEndpointDiscovery,
bool enableEndpointDiscovery,
bool isThinClientEnabled)
{
this.throttlingRetry = new ResourceThrottleRetryPolicy(
Expand All @@ -75,7 +75,7 @@ public ClientRetryPolicy(
this.sessionTokenRetryCount = 0;
this.serviceUnavailableRetryCount = 0;
this.canUseMultipleWriteLocations = false;
this.isMultiMasterWriteRequest = false;
this.isMultiMasterWriteRequest = false;
this.isThinClientEnabled = isThinClientEnabled;
}

Expand Down Expand Up @@ -248,7 +248,7 @@ 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 !INTERNAL
// Initialize CrossRegionAvailabilityContext from Properties if not already set.
// In hedging scenarios, Properties carries the shared context instance injected by
Expand Down Expand Up @@ -277,6 +277,36 @@ public void OnBeforeSendRequest(DocumentServiceRequest request)
: this.globalEndpointManager.ResolveServiceEndpoint(request);

request.RequestContext.RouteToLocation(this.locationEndpoint);

// Hedging-Detection API: tag the upcoming dispatch reason on Properties so that
// the downstream dispatch site (TransportHandler / GatewayStoreModel) can append
// a RequestedRegion entry with the correct reason. Only override when this is a
// genuine retry attempt — first-attempt dispatches default to Initial (set by
// the dispatch site), and hedge-arm dispatches have their Hedging reason set by
// CrossRegionHedgingAvailabilityStrategy before reaching this policy.
//
// Hedging preservation invariant: when a hedge arm itself triggers a retry (e.g.
// 410 Gone / 449), this method is re-entered with retryContext != null on the
// same cloned RequestMessage. The Hedging value previously seeded by
// CrossRegionHedgingAvailabilityStrategy.CloneAndSendAsync must NOT be silently
// overwritten with OperationRetry / RegionFailover, otherwise the hedge origin
// is lost from the GetRequestedRegions() sequence. Preserve the existing value
// if it is already Hedging.
if (this.retryContext != null && request.Properties != null)
Comment thread
kundadebdatta marked this conversation as resolved.
{
bool alreadyTaggedAsHedging =
request.Properties.TryGetValue(Tracing.HedgingDetectionState.DispatchReasonPropertyKey, out object existingReasonObj)
&& existingReasonObj is RequestedRegionReason existingReason
&& existingReason == RequestedRegionReason.Hedging;

if (!alreadyTaggedAsHedging)
{
RequestedRegionReason reason = this.retryContext.RetryRequestOnPreferredLocations
? RequestedRegionReason.RegionFailover
: RequestedRegionReason.OperationRetry;
request.Properties[Tracing.HedgingDetectionState.DispatchReasonPropertyKey] = reason;
}
}
}

private async Task<ShouldRetryResult> ShouldRetryInternalAsync(
Expand Down Expand Up @@ -371,14 +401,14 @@ private async Task<ShouldRetryResult> ShouldRetryInternalAsync(
markBothReadAndWriteAsUnavailable: false,
forceRefresh: false,
retryOnPreferredLocations: true);
}
}

if (statusCode == HttpStatusCode.NotFound && subStatusCode == SubStatusCodes.ReadSessionNotAvailable)
{
return this.ShouldRetryOnSessionNotAvailable(this.documentServiceRequest);
}
// Received 503 due to client connect timeout or Gateway
}

// Received 503 due to client connect timeout or Gateway
if (statusCode == HttpStatusCode.ServiceUnavailable)
{
return this.TryMarkEndpointUnavailableForPkRangeAndRetryOnServiceUnavailable(
Expand Down
95 changes: 95 additions & 0 deletions Microsoft.Azure.Cosmos/src/Diagnostics/CosmosDiagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -97,5 +97,100 @@ public virtual ServerSideCumulativeMetrics GetQueryMetrics()
/// The returned list contains unique regions and doesn't guarantee ordering of the regions contacted from the first to the last
/// </remarks>
public abstract IReadOnlyList<(string regionName, Uri uri)> GetContactedRegions();

/// <summary>
/// Returns <c>true</c> if the SDK actually dispatched this operation to a hedge
/// region as part of a cross-region availability-strategy fan-out. Returns
/// <c>false</c> for non-hedged operations and for hedged operations whose primary
/// responded under the configured threshold (hedge tasks were registered but never
/// awaited; no fan-out occurred).
/// </summary>
/// <returns>
/// <c>true</c> if at least one hedge arm was actually dispatched; otherwise <c>false</c>.
/// </returns>
/// <remarks>
/// <para>
/// <b>A return value of <c>false</c> does NOT mean hedging was disabled or
/// misconfigured.</b> To check whether hedging was <i>configured</i> on the client,
/// inspect <c>CosmosClientOptions.AvailabilityStrategy</c> or
/// <c>RequestOptions.AvailabilityStrategy</c> directly.
/// </para>
/// <para>
/// O(1). Safe to call on both the success path (<c>ItemResponse&lt;T&gt;.Diagnostics</c>)
/// and the error path (<c>CosmosException.Diagnostics</c>). Returns <c>false</c> for
/// diagnostics objects created outside the SDK pipeline (e.g. customer-authored
/// subclasses of <see cref="CosmosDiagnostics"/>).
/// </para>
/// </remarks>
public virtual bool HedgingStarted()
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Recommendation (F2) No end-to-end tests cover the wiring this PR adds.

The 17 added tests cover HedgingDetectionState and RequestedRegion in isolation plus AC9 backward-compat defaults — but nothing verifies the integration that is the entire ask of the PR:

  • ClientRetryPolicy.OnBeforeSendRequest actually writes OperationRetry vs RegionFailover for the two branches of RetryRequestOnPreferredLocations.
  • CrossRegionHedgingAvailabilityStrategy.CloneAndSendAsync writes Hedging only on requestNumber > 0.
  • TransportHandler.AppendDispatchedRegion resolves a region, appends an entry, and removes the property.
  • An end-to-end hedge fan-out via the existing mock pipeline produces the expected HedgingStarted()/GetRequestedRegions()/GetRespondedRegions() sequence.

AC1–AC5 are essentially asserted by inspection only. CrossRegionHedgingAvailabilityStrategyTests is the natural home for the integration coverage, and the gap is what would have caught F3 (hedge-arm retry overwrite) before merge.

{
return false;
}

/// <summary>
/// Returns the regions the SDK actually dispatched this operation to, in observed
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit - thoughts on using verbiage like CosmosClient dispatched this operation to ...

/// dispatch order, each tagged with the reason the SDK chose it. Includes the
/// initial attempt.
/// </summary>
/// <returns>
/// A read-only list of <see cref="RequestedRegion"/> entries. Never <c>null</c>;
/// returns an empty list when there is no dispatch history (for example, when a
/// pre-flight client-side validation failed before any dispatch was attempted, or
/// for diagnostics objects created outside the SDK pipeline).
/// </returns>
/// <remarks>
/// <para>
/// The append site is the actual dispatch point; registered-but-never-awaited
/// hedge tasks do <b>not</b> appear here. <see cref="RequestedRegionReason"/> is
/// non-exhaustive — callers MUST include a <c>default</c> arm when switching on
/// the enum to handle future values.
/// </para>
/// <para>
/// <b>Contract is "dispatched, not necessarily wire-issued".</b> An entry reflects
/// the SDK's decision to commit to dispatching — for hedge arms specifically, this
/// means the per-arm threshold delay elapsed without cancellation. A cancellation
/// arriving in the microsecond-wide window between that dispatch decision and the
/// underlying HTTP/RNTBD send leaving the process still leaves the entry in this
/// list. Callers should treat the list as a record of intent-to-dispatch, not a
/// record of wire-issued requests.
/// </para>
/// <para>
/// Duplicates are allowed (the same region can be tried multiple times across
/// retries, fail-overs and probes). Returns empty for SDK clients constructed
/// before this version's diagnostics plumbing populated it. Available on success
/// and error paths.
/// </para>
/// </remarks>
public virtual IReadOnlyList<RequestedRegion> GetRequestedRegions()
{
return Array.Empty<RequestedRegion>();
}

/// <summary>
/// Returns the regions that responded to this operation (success or failure), in
/// arrival order as observed by the SDK orchestrator.
/// </summary>
/// <returns>
/// A read-only list of region names. Never <c>null</c>; returns an empty list when
/// no response arrived (for example, when all dispatches were cancelled or timed
/// out) or for diagnostics objects created outside the SDK pipeline.
/// </returns>
/// <remarks>
/// <para>
/// <b>Duplicates are allowed and expected.</b> The same region may appear more
/// than once if it produced multiple responses (e.g., a late response after a
/// hedge winner). <c>Count &gt; 1</c> does <b>not</b> imply that more than one
/// distinct region responded. For unique regions, call
/// <c>.Distinct(StringComparer.OrdinalIgnoreCase)</c>.
/// </para>
/// <para>
/// Returns empty for SDK clients constructed before this version's diagnostics
/// plumbing populated it. Available on success and error paths.
/// </para>
/// </remarks>
public virtual IReadOnlyList<string> GetRespondedRegions()
{
return Array.Empty<string>();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -158,5 +158,25 @@ public override int GetFailedRequestCount()

return this.Value.Summary.GetFailedCount();
}

/// <inheritdoc/>
public override bool HedgingStarted()
{
return this.Value?.Summary?.HedgingDetectionState?.HedgingStarted ?? false;
}

/// <inheritdoc/>
public override IReadOnlyList<RequestedRegion> GetRequestedRegions()
{
return this.Value?.Summary?.HedgingDetectionState?.GetRequestedRegionsSnapshot()
Copy link
Copy Markdown
Member

@jeet1995 jeet1995 May 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would debugging work when the result is empty? When all is empty outcome expected?

?? Array.Empty<RequestedRegion>();
}

/// <inheritdoc/>
public override IReadOnlyList<string> GetRespondedRegions()
{
return this.Value?.Summary?.HedgingDetectionState?.GetRespondedRegionsSnapshot()
?? Array.Empty<string>();
}
}
}
103 changes: 103 additions & 0 deletions Microsoft.Azure.Cosmos/src/Diagnostics/RequestedRegion.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
//------------------------------------------------------------
// Copyright (c) Microsoft Corporation. All rights reserved.
//------------------------------------------------------------
namespace Microsoft.Azure.Cosmos
{
using System;

/// <summary>
/// Represents a single region the SDK dispatched a request to as part of an operation,
/// tagged with the reason the orchestrator chose to send it.
/// </summary>
/// <remarks>
/// <para>Used by <see cref="CosmosDiagnostics.GetRequestedRegions"/> to enumerate every
/// dispatched attempt in observed dispatch order, including the initial attempt and any
/// retries, region fail-overs, hedge arms, or circuit-breaker probes.</para>
/// <para>This value is immutable. Equality is case-insensitive on
/// <see cref="RegionName"/> and exact on <see cref="Reason"/>.</para>
/// </remarks>
public readonly struct RequestedRegion : IEquatable<RequestedRegion>
{
/// <summary>
/// Initializes a new <see cref="RequestedRegion"/>.
/// </summary>
/// <param name="regionName">
/// The name of the region the SDK dispatched to (e.g. "East US"). Must not be null.
/// </param>
/// <param name="reason">The reason the SDK chose this region for this dispatch.</param>
/// <exception cref="ArgumentNullException"><paramref name="regionName"/> is null.</exception>
public RequestedRegion(string regionName, RequestedRegionReason reason)
{
this.RegionName = regionName ?? throw new ArgumentNullException(nameof(regionName));
this.Reason = reason;
}

/// <summary>
/// Gets the name of the region the SDK dispatched to.
/// </summary>
public string RegionName { get; }

/// <summary>
/// Gets the reason the SDK chose this region for this particular dispatch attempt.
/// </summary>
public RequestedRegionReason Reason { get; }

/// <summary>
/// Determines whether this instance equals another <see cref="RequestedRegion"/> by
/// comparing <see cref="RegionName"/> case-insensitively and <see cref="Reason"/> exactly.
/// </summary>
/// <param name="other">The other <see cref="RequestedRegion"/> to compare against.</param>
/// <returns><c>true</c> when both region names match (case-insensitive) and the reasons match exactly; otherwise <c>false</c>.</returns>
public bool Equals(RequestedRegion other)
{
return string.Equals(this.RegionName, other.RegionName, StringComparison.OrdinalIgnoreCase)
&& this.Reason == other.Reason;
}

/// <inheritdoc/>
public override bool Equals(object obj)
{
return obj is RequestedRegion other && this.Equals(other);
}

/// <inheritdoc/>
public override int GetHashCode()
{
int regionHash = this.RegionName == null
? 0
: StringComparer.OrdinalIgnoreCase.GetHashCode(this.RegionName);
// Combine without depending on System.HashCode (net6+) to keep this constant
// across TFMs.
unchecked
{
return (regionHash * 397) ^ (byte)this.Reason;
Comment thread
kundadebdatta marked this conversation as resolved.
}
}

/// <summary>
/// Returns a human-readable representation of this <see cref="RequestedRegion"/> in the
/// form <c>"{regionName}:{reason}"</c>.
/// </summary>
/// <returns>A string of the form <c>"{regionName}:{reason}"</c>.</returns>
public override string ToString()
{
return $"{this.RegionName}:{this.Reason}";
}

/// <summary>
/// Equality operator.
/// </summary>
/// <param name="left">The left operand.</param>
/// <param name="right">The right operand.</param>
/// <returns><c>true</c> if the two values are equal; otherwise <c>false</c>.</returns>
public static bool operator ==(RequestedRegion left, RequestedRegion right) => left.Equals(right);

/// <summary>
/// Inequality operator.
/// </summary>
/// <param name="left">The left operand.</param>
/// <param name="right">The right operand.</param>
/// <returns><c>true</c> if the two values are not equal; otherwise <c>false</c>.</returns>
public static bool operator !=(RequestedRegion left, RequestedRegion right) => !left.Equals(right);
}
}
Loading
Loading