diff --git a/sdk/cosmos/azure_data_cosmos/docs/HEDGING_DETECTION_API_SPEC.md b/sdk/cosmos/azure_data_cosmos/docs/HEDGING_DETECTION_API_SPEC.md new file mode 100644 index 00000000000..5dcba0788a7 --- /dev/null +++ b/sdk/cosmos/azure_data_cosmos/docs/HEDGING_DETECTION_API_SPEC.md @@ -0,0 +1,140 @@ +# Hedging Detection — Diagnostics Surface Spec + +**Status:** Implemented in this PR, targeting `main`. +**Target branch:** `main` +**Tracking issue:** [Azure/azure-sdk-for-rust#4410](https://github.com/Azure/azure-sdk-for-rust/issues/4410) +**Cross-SDK contract:** The Cosmos DB SDKs are converging on a "Hedging Detection" capability exposed on each SDK's per-operation diagnostics. This document specifies how the Rust SDK satisfies that capability. + +--- + +## 1. Summary + +The Rust Cosmos SDK already exposes everything needed to detect hedging and to reconstruct per-region dispatch/response history from a single `DiagnosticsContext`. Rather than ship a parallel set of computed convenience helpers (`hedging_started()`, `requested_regions()`, `responded_regions()`) and a parallel `RequestedRegion` / `RequestedRegionReason` model, this PR keeps the v1 public surface deliberately small and lets callers read the already-public building blocks directly. + +This decision follows reviewer feedback on [PR #4558](https://github.com/Azure/azure-sdk-for-rust/pull/4558): convenience helpers that "dig through the requests and create new data" force allocations a caller may not want, and lock in an API shape before we know how customers consume it. Cosmos is pre-1.0, so we prefer to be conservative now and add helpers later (e.g., when we build loggable diagnostic strings) once usage patterns are clear. + +The only code change this PR makes to the diagnostics surface is renaming the internal `ExecutionContext::Retry` reason to `ExecutionContext::OperationRetry` (§3), which clarifies the per-request "why" that the detection recipes in §4 rely on. + +--- + +## 2. Building blocks (already public) + +All citations are SHA-pinned to the `main` commit at the merge of the hedging design spec ([PR #4330](https://github.com/Azure/azure-sdk-for-rust/pull/4330)), commit [`5f5d8c49d02b579a2afd2297857b919900ff1dad`](https://github.com/Azure/azure-sdk-for-rust/commit/5f5d8c49d02b579a2afd2297857b919900ff1dad), file [`diagnostics_context.rs`](https://github.com/Azure/azure-sdk-for-rust/blob/5f5d8c49d02b579a2afd2297857b919900ff1dad/sdk/cosmos/azure_data_cosmos_driver/src/diagnostics/diagnostics_context.rs). Current line numbers may have drifted; the accessors and their semantics are stable. + +| Item | Signature | Notes | +| --- | --- | --- | +| `DiagnosticsContext` | re-exported as `azure_data_cosmos::DiagnosticsContext` | The per-operation diagnostics handle. | +| `DiagnosticsContext::requests()` | `-> Arc>` | All dispatched attempts, in **dispatch order** (append-only). Cloning the `Arc` is a cheap atomic increment. | +| `DiagnosticsContext::hedge_diagnostics()` | `-> Option<&HedgeDiagnostics>` | `Some` whenever a hedging strategy was active for the operation (including primary-wins-under-threshold). | +| `DiagnosticsContext::regions_contacted()` | `-> Vec` | **Sorted and deduplicated** distinct regions — not dispatch order. | +| `RequestDiagnostics::region()` | `-> Option<&Region>` | `None` for pre-region-selection failures. | +| `RequestDiagnostics::execution_context()` | `-> ExecutionContext` | Why this attempt was dispatched (see §3). | +| `RequestDiagnostics::completed_at()` | `-> Option` | Set by `complete()`, `timeout()`, **and** `fail_transport()` — so "completed" alone is not "responded" (see §4.2). | +| `RequestDiagnostics::timed_out()` | `-> bool` | `true` for a client-side end-to-end timeout. | +| `HedgeDiagnostics::alternate_region()` | `-> Option<&Region>` | `Some` exactly when the orchestrator dispatched an alternate hedge leg (i.e., fan-out happened). | +| `HedgeDiagnostics::response_region()` | `-> Option<&Region>` | The single winning region, when a leg won. | +| `HedgeDiagnostics::terminal_state()` | `-> HedgeTerminalState` | Authoritative race outcome. | + +Because these are inherent methods on the driver's `DiagnosticsContext` and the SDK depends on the driver (never the reverse), the diagnostics model is driver-owned and re-exported by `azure_data_cosmos`, exactly like `DiagnosticsContext` itself. + +--- + +## 3. The `Retry → OperationRetry` rename + +`ExecutionContext` is the per-request "why" returned by `RequestDiagnostics::execution_context()`: + +```rust +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Serialize)] +#[serde(rename_all = "snake_case")] +#[non_exhaustive] +pub enum ExecutionContext { + Initial, + OperationRetry, // was: Retry + TransportRetry, + Hedging, + RegionFailover, + CircuitBreakerProbe, +} +``` + +This PR renames `Retry` to `OperationRetry` so the operation-level retry reason is clearly distinct from the transport-level `TransportRetry`. The hand-written `ExecutionContext::as_str()` and every dispatch site (`operation_pipeline.rs`, `transport_pipeline.rs`) are updated accordingly. + +**Wire-format note.** The serialized form changes from `"retry"` to `"operation_retry"`. Cosmos is pre-1.0, so the old variant is removed outright rather than kept as a `#[deprecated]` alias — there is no enum-variant alias mechanism in Rust, and aliasing a soon-to-change wire string adds churn without value. Telemetry parsers that match on the literal `"retry"` must update. (`ExecutionContext` derives `Serialize` only, not `Deserialize`, so no `#[serde(alias)]` is needed.) + +--- + +## 4. Detection recipes + +These are the exact computations a caller runs against the building blocks in §2. They are documented here (rather than shipped as methods) so callers allocate only when they actually need a derived collection. + +### 4.1 Did fan-out happen? (`hedging_started`) + +`true` iff at least one hedge arm was actually dispatched. This is `false` — not an error — when the primary returns before the hedging threshold elapses, even though a hedging strategy was active. + +```rust +fn hedging_started(ctx: &DiagnosticsContext) -> bool { + ctx.hedge_diagnostics() + .map(|hd| hd.alternate_region().is_some()) + .unwrap_or(false) + || ctx + .requests() + .iter() + .any(|r| matches!(r.execution_context(), ExecutionContext::Hedging)) +} +``` + +The two signals — `HedgeDiagnostics::alternate_region().is_some()` and any request tagged `ExecutionContext::Hedging` — are equivalent on `main` ([#4432](https://github.com/Azure/azure-sdk-for-rust/pull/4432)). Reading either is sufficient; the disjunction stays correct if a future change ever drifts one. To check whether a hedging strategy was merely *configured* (a superset that includes primary-wins-under-threshold), use `ctx.hedge_diagnostics().is_some()`. + +### 4.2 Regions dispatched to, with reason (`requested_regions`) + +Dispatch order, duplicates preserved (a region dispatched twice appears twice), entries with no resolved region skipped: + +```rust +let dispatched: Vec<(Region, ExecutionContext)> = ctx + .requests() + .iter() + .filter_map(|r| r.region().map(|reg| (reg.clone(), r.execution_context()))) + .collect(); +``` + +`ExecutionContext` carries the reason directly, so no separate `RequestedRegionReason` enum is needed — the request's own context *is* the reason. This is distinct from `regions_contacted()`, which is sorted and deduplicated. + +### 4.3 Regions that responded (`responded_regions`) + +A region "responded" only if a service reply actually arrived. `completed_at` is **not** a sufficient filter: the driver also sets it for client-side timeouts (`timeout()`) and transport failures (`fail_transport()`). The correct predicate excludes those two cases; a non-2xx HTTP status (404/429) still counts as a response: + +```rust +let mut responded: Vec<&RequestDiagnostics> = ctx + .requests() + .iter() + .filter(|r| { + r.region().is_some() && r.completed_at().is_some() && !r.timed_out() && r.error().is_none() + }) + .collect(); +// Arrival order, preserving dispatch order among ties. +responded.sort_by_key(|r| r.completed_at()); +let responded_regions: Vec<&Region> = responded.iter().filter_map(|r| r.region()).collect(); +``` + +Duplicates are preserved (e.g., a late hedge loser after the winner). To deduplicate, collect into a `BTreeSet`. + +--- + +## 5. Reconciliation with `HedgeDiagnostics` + +The cross-SDK detection recipes and the Rust-native `HedgeDiagnostics` ([PR #4330](https://github.com/Azure/azure-sdk-for-rust/pull/4330) design, [#4432](https://github.com/Azure/azure-sdk-for-rust/pull/4432) implementation; authoritative shape in [`HEDGING_SPEC.md`](https://github.com/Azure/azure-sdk-for-rust/blob/5f5d8c49d02b579a2afd2297857b919900ff1dad/sdk/cosmos/azure_data_cosmos_driver/docs/HEDGING_SPEC.md)) coexist on the same `DiagnosticsContext` and serve different audiences. + +| Question | Recipe (§4) | Rust-native `HedgeDiagnostics` | +| --- | --- | --- | +| Did fan-out happen? | §4.1 | `hedge_diagnostics().map(\|hd\| hd.alternate_region().is_some()).unwrap_or(false)` — equivalent | +| Was a strategy active? | *(not derived)* | `hedge_diagnostics().is_some()` — superset of fan-out | +| Regions tried | §4.2 (every attempt, with reason) | `primary_region()` + `alternate_region()` (hedge legs only) | +| Regions that responded | §4.3 (full list, completion order) | `response_region()` (single winner) | + +`main`'s `HedgeDiagnostics` classifies the race via `terminal_state` / `alternate_region` (no `total_requests_launched` counter), so "fan-out happened" is `alternate_region().is_some()` and "the alternate won" is `matches!(terminal_state(), HedgeTerminalState::AlternateWon)`. + +--- + +## 6. Future work + +If usage shows callers repeatedly hand-rolling §4, we can revisit adding first-class helpers — most naturally when building concise, loggable diagnostic-string output, where the allocation is already paid for. Any such helper should land with a clear consumer rather than speculatively. Likewise, `ExecutionContext` could be renamed to something friendlier (e.g., `RequestPurpose` / `RequestIntent`) if it becomes a prominent part of the public detection surface; that rename is out of scope for this PR. diff --git a/sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md b/sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md index 50707015d63..867e79e1908 100644 --- a/sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md +++ b/sdk/cosmos/azure_data_cosmos_driver/CHANGELOG.md @@ -60,6 +60,7 @@ ### Breaking Changes +- Renamed `ExecutionContext::Retry` to `ExecutionContext::OperationRetry` to distinguish SDK operation-level retries from transport-level retries (`ExecutionContext::TransportRetry`). The serialized form changes from `"retry"` to `"operation_retry"`. ([#4558](https://github.com/Azure/azure-sdk-for-rust/pull/4558)) - Renamed the error surface: `Error` → `CosmosError`, `ErrorBuilder` → `CosmosErrorBuilder`. Categorization moved from a `Kind` enum to predicates on `CosmosStatus` (`is_not_found()`, `is_throttled()`, `is_transient()`, …); error details are reached via `status()` and `response()` instead of the previous flat accessors. ([#4442](https://github.com/Azure/azure-sdk-for-rust/pull/4442)) - Renamed `MaxItemCount` to `MaxItemCountHint` to better reflect that the value is a hint to the service (which may return fewer items) rather than a strict cap. The SDK already exposed the type under the new name via a `use ... as MaxItemCountHint`; the rename makes the canonical name consistent across both crates. Update callers that reference `azure_data_cosmos_driver::models::MaxItemCount`, `CosmosRequestHeaders::max_item_count` typings, or `CosmosOperation::with_max_item_count` argument types accordingly. - Marked `PartitionKeyVersion` and `CosmosStatus` as `#[non_exhaustive]` to allow future variants/fields to be added without further breaking changes. Callers must use `..` wildcard arms when matching on `PartitionKeyVersion`; `CosmosStatus` already had private fields and is constructed via `CosmosStatus::new` / `with_sub_status`, so the attribute is primarily a forward-compat signal. diff --git a/sdk/cosmos/azure_data_cosmos_driver/src/diagnostics/diagnostics_context.rs b/sdk/cosmos/azure_data_cosmos_driver/src/diagnostics/diagnostics_context.rs index 36b9bba9454..3bf3e29cc2c 100644 --- a/sdk/cosmos/azure_data_cosmos_driver/src/diagnostics/diagnostics_context.rs +++ b/sdk/cosmos/azure_data_cosmos_driver/src/diagnostics/diagnostics_context.rs @@ -35,8 +35,11 @@ use std::{ pub enum ExecutionContext { /// Initial request attempt (first try). Initial, - /// Retry due to transient error (e.g., 429, 503). - Retry, + /// An operation-level retry decided by the SDK's client-retry policy. + /// + /// Distinguishes user-visible operation retries from transport-layer + /// retries ([`ExecutionContext::TransportRetry`]). + OperationRetry, /// Transport-level shard retry within the same region. /// /// The initial attempt failed with a connectivity error and the transport @@ -56,7 +59,7 @@ impl ExecutionContext { pub fn as_str(&self) -> &'static str { match self { ExecutionContext::Initial => "initial", - ExecutionContext::Retry => "retry", + ExecutionContext::OperationRetry => "operation_retry", ExecutionContext::TransportRetry => "transport_retry", ExecutionContext::Hedging => "hedging", ExecutionContext::RegionFailover => "region_failover", @@ -2244,7 +2247,7 @@ mod tests { builder.update_request(h1, |req| req.request_charge = RequestCharge::new(3.0)); let h2 = builder.start_test_request( - ExecutionContext::Retry, + ExecutionContext::OperationRetry, Some(Region::WEST_US_2), "https://test.documents.azure.com", ); @@ -2263,7 +2266,7 @@ mod tests { "https://test.westus2.documents.azure.com", ); builder.start_test_request( - ExecutionContext::Retry, + ExecutionContext::OperationRetry, Some(Region::WEST_US_2), "https://test.westus2.documents.azure.com", ); @@ -2488,7 +2491,7 @@ mod tests { // Add several requests to trigger deduplication for i in 0..5 { let handle = builder.start_test_request( - ExecutionContext::Retry, + ExecutionContext::OperationRetry, Some(Region::WEST_US_2), "https://test.documents.azure.com", ); @@ -2515,7 +2518,7 @@ mod tests { "request_count": 5, "total_request_charge": 10.0, "first": { - "execution_context": "retry", + "execution_context": "operation_retry", "endpoint": "https://test.documents.azure.com/", "status": "429/3200 (RUBudgetExceeded)", "request_charge": 0.0, @@ -2523,7 +2526,7 @@ mod tests { "timed_out": false }, "last": { - "execution_context": "retry", + "execution_context": "operation_retry", "endpoint": "https://test.documents.azure.com/", "status": "429/3200 (RUBudgetExceeded)", "request_charge": 4.0, @@ -2533,8 +2536,7 @@ mod tests { "deduplicated_groups": [{ "endpoint": "https://test.documents.azure.com/", "status": "429/3200 (RUBudgetExceeded)", - "execution_context": "retry", - + "execution_context": "operation_retry", "count": 3, "total_request_charge": 6.0, "min_duration_ms": 0, @@ -2708,7 +2710,10 @@ mod tests { #[test] fn execution_context_display() { assert_eq!(ExecutionContext::Initial.to_string(), "initial"); - assert_eq!(ExecutionContext::Retry.to_string(), "retry"); + assert_eq!( + ExecutionContext::OperationRetry.to_string(), + "operation_retry" + ); assert_eq!( ExecutionContext::TransportRetry.to_string(), "transport_retry" diff --git a/sdk/cosmos/azure_data_cosmos_driver/src/driver/pipeline/operation_pipeline.rs b/sdk/cosmos/azure_data_cosmos_driver/src/driver/pipeline/operation_pipeline.rs index a66c419d085..1b6017a38d8 100644 --- a/sdk/cosmos/azure_data_cosmos_driver/src/driver/pipeline/operation_pipeline.rs +++ b/sdk/cosmos/azure_data_cosmos_driver/src/driver/pipeline/operation_pipeline.rs @@ -1476,7 +1476,7 @@ fn compute_execution_context(retry_state: &OperationRetryState) -> ExecutionCont if retry_state.failover_retry_count == 0 && retry_state.session_token_retry_count == 0 { ExecutionContext::Initial } else if retry_state.session_token_retry_count > 0 { - ExecutionContext::Retry + ExecutionContext::OperationRetry } else { ExecutionContext::RegionFailover } @@ -3674,7 +3674,7 @@ mod tests { let ctx = TransportRequestContext { routing: &routing, activity_id: &activity_id, - execution_context: ExecutionContext::Retry, + execution_context: ExecutionContext::OperationRetry, deadline: Some(std::time::Instant::now() + Duration::from_secs(5)), resolved_session_token: None, throughput_control: None, @@ -5779,13 +5779,13 @@ mod tests { let state = retry_state_with_counts(1, 1); assert!(matches!( super::compute_execution_context(&state), - ExecutionContext::Retry + ExecutionContext::OperationRetry )); let state = retry_state_with_counts(0, 1); assert!(matches!( super::compute_execution_context(&state), - ExecutionContext::Retry + ExecutionContext::OperationRetry )); } diff --git a/sdk/cosmos/azure_data_cosmos_driver/src/driver/transport/transport_pipeline.rs b/sdk/cosmos/azure_data_cosmos_driver/src/driver/transport/transport_pipeline.rs index 54627bd15a4..79a7da34014 100644 --- a/sdk/cosmos/azure_data_cosmos_driver/src/driver/transport/transport_pipeline.rs +++ b/sdk/cosmos/azure_data_cosmos_driver/src/driver/transport/transport_pipeline.rs @@ -205,7 +205,7 @@ pub(crate) async fn execute_transport_pipeline( } else if throttle_state.attempt_count == 0 { request.execution_context } else { - ExecutionContext::Retry + ExecutionContext::OperationRetry }; let request_handle = diagnostics.start_request(