[Cosmos] Refresh Hedging Detection API by NaluTripician · Pull Request #4558 · Azure/azure-sdk-for-rust

NaluTripician · 2026-06-09T20:42:07Z

Implements the Hedging Detection API for the Rust Cosmos SDK and lands the accompanying spec (sdk/cosmos/azure_data_cosmos/docs/HEDGING_DETECTION_API_SPEC.md) in the same PR — the change was small enough to combine. Rebased onto current main, which already carries the hedging implementation (#4432) and the hedging design spec (#4330).

What's implemented

Driver (azure_data_cosmos_driver)

Added the cross-SDK Hedging Detection API accessors on DiagnosticsContext:
- requested_regions() -> Vec<RequestedRegion> — dispatch order, duplicates allowed, region-None entries skipped, each tagged with a RequestedRegionReason.
- responded_regions() -> Vec<&Region> — completion order (stable sort by completed_at), service replies only (excludes client timeouts and transport failures via a new RequestDiagnostics::responded_with_service_reply() predicate; non-2xx replies still count).
- hedging_started() -> bool — disjunction of hedge_diagnostics().alternate_region().is_some() (fan-out per main's landed HedgeDiagnostics) and any RequestDiagnostics tagged ExecutionContext::Hedging.
Added the public RequestedRegion struct and RequestedRegionReason enum (both #[non_exhaustive]) plus a total From<ExecutionContext> mapping. TransportRetry / CircuitBreakerProbe reasons are reserved.
Renamed ExecutionContext::Retry → ExecutionContext::OperationRetry (old variant kept #[deprecated] for one release; serialized string changes "retry" → "operation_retry"). Updated as_str() and all dispatch sites.

SDK (azure_data_cosmos)

Re-exported RequestedRegion / RequestedRegionReason, mirroring the existing DiagnosticsContext re-export.

Docs / CHANGELOGs

Reconciled the spec with main's reality: DiagnosticsContext naming (not the previews-era CosmosDiagnosticsContext), HedgeDiagnostics shape (terminal_state / alternate_region, not total_requests_launched / was_hedge), and the now-landed hedging implementation (Cosmos: Implement Cross Regional Hedging In Driver Crate #4432).
Added CHANGELOG entries to both crates.

Validation

cargo build, cargo clippy, and cargo fmt --check are clean on both crates; the full driver lib test suite passes (1771 tests), including new unit tests for the three accessors and the From<ExecutionContext> mapping totality. No new markdownlint errors introduced.

Refs #4410.

cc @kundadebdatta @analogrelay @FabianMeiswinkel @simorenoh

Copilot

Pull request overview

This PR adds a refreshed spec-only Hedging Detection API contract document for the Rust Cosmos SDK (azure_data_cosmos), intended to replace the previously truncated spec and align with the merged hedging design spec (#4330) plus the cross-SDK contract.

Changes:

Adds a new Hedging Detection API spec covering contract semantics, proposed API surface, and Rust↔contract mapping.
Specifies per-dispatch region “reason” via RequestedRegion { region, reason } / RequestedRegionReason, plus the Retry → OperationRetry rename mechanics.
Includes updated reconciliation against HedgeDiagnostics, plus test-plan and CHANGELOG draft sections.

Resolve the valid issues raised by the Copilot PR review on Azure#4558: - Crate layering: the accessors are inherent methods on the driver's DiagnosticsContext, but azure_data_cosmos depends on azure_data_cosmos_driver (never the reverse), so the driver cannot return SDK-defined types. The spec now defines RequestedRegion and RequestedRegionReason in the driver and re-exports them from azure_data_cosmos (the same pattern as DiagnosticsContext). Reworked section 5 into a three-option comparison (re-export ExecutionContext, driver-defined+re-exported types, SDK-side extension trait) and reframed the 'No Model Sharing' discussion accordingly. - responded_regions filter: completed_at is set by complete(), timeout(), and fail_transport(), so completed_at.is_some() alone wrongly includes timeouts and transport failures. The filter now also requires !timed_out() and error.is_none(). - Added SHA-pinned permalinks for the section 2 surface inventory and the section 6 HedgeDiagnostics citation, per the tracking issue's auditability criteria. Spec-only; no production code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Reconcile sdk/cosmos/azure_data_cosmos/docs/HEDGING_DETECTION_API_SPEC.md with the now-merged Rust hedging design spec (PR Azure#4330, on release/azure_data_cosmos-previews) and align it with the cross-SDK Hedging Detection API contract. Key updates: - Describe the cross-SDK contract generically (no SDK-specific naming) and add a Rust-to-contract mapping table. - requested_regions() now returns Vec<RequestedRegion> carrying a RequestedRegionReason, conforming to the contract's per-region-reason shape; add the public RequestedRegion struct and RequestedRegionReason enum plus a From<ExecutionContext> mapping. This resolves the previously-deferred per-region-reason question. - Correct the Retry to OperationRetry rename mechanics to the real code: ExecutionContext derives Serialize only (no Deserialize), and the hand-written as_str() must also be updated; serde(alias) is now conditional on adding Deserialize. - Fix the HedgeDiagnostics reconciliation: strategy_config and response_region are non-optional in the merged HEDGING_SPEC.md; fix the hedging_started() doc-comment that called as_ref() on a non-Option. - Correct the surface inventory: real dispatch-site paths, Serialize-only derive, and that regions_contacted() is sorted/deduped (so new accessors are needed for dispatch/completion order with duplicates). - Complete the file, which was truncated mid-section 8.2: finish the test plan and add section 9 (CHANGELOG drafts) and section 10 (open questions). Spec-only; no production code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Resolve the valid issues raised by the Copilot PR review on Azure#4558: - Crate layering: the accessors are inherent methods on the driver's DiagnosticsContext, but azure_data_cosmos depends on azure_data_cosmos_driver (never the reverse), so the driver cannot return SDK-defined types. The spec now defines RequestedRegion and RequestedRegionReason in the driver and re-exports them from azure_data_cosmos (the same pattern as DiagnosticsContext). Reworked section 5 into a three-option comparison (re-export ExecutionContext, driver-defined+re-exported types, SDK-side extension trait) and reframed the 'No Model Sharing' discussion accordingly. - responded_regions filter: completed_at is set by complete(), timeout(), and fail_transport(), so completed_at.is_some() alone wrongly includes timeouts and transport failures. The filter now also requires !timed_out() and error.is_none(). - Added SHA-pinned permalinks for the section 2 surface inventory and the section 6 HedgeDiagnostics citation, per the tracking issue's auditability criteria. Spec-only; no production code changes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Land the Hedging Detection API implementation described by HEDGING_DETECTION_API_SPEC.md in the same PR as the spec, since the change is small enough to combine. Driver (azure_data_cosmos_driver): - Add `OperationRetry` to `ExecutionContext`; deprecate `Retry` (serialized form changes "retry" -> "operation_retry"). Update `as_str()` and all dispatch sites in the operation/transport pipelines. - Add public `RequestedRegion` struct and `RequestedRegionReason` enum (both `#[non_exhaustive]`) plus a total `From<ExecutionContext>` mapping. - Add `DiagnosticsContext::requested_regions()` (dispatch order, duplicates, per-region reason), `responded_regions()` (completion order, service replies only via a new `responded_with_service_reply()` predicate), and `hedging_started()` (ExecutionContext::Hedging predicate; the hedge_diagnostics signal lights up once the hedging implementation lands). - Export the two new types from the diagnostics module. - Add unit tests for the accessors and mapping totality. SDK (azure_data_cosmos): - Re-export `RequestedRegion` / `RequestedRegionReason`, mirroring `CosmosDiagnosticsContext`. Docs/CHANGELOGs: - Reconcile the spec's "spec-only" framing now that the implementation lands together, and add CHANGELOG entries to both crates. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

analogrelay · 2026-06-11T22:51:03Z

 - Exposed cross-regional read hedging. Enable it by attaching an `OperationOptions` built with `OperationOptionsBuilder::with_availability_strategy(AvailabilityStrategy::Hedging(HedgingStrategy::new(HedgeThreshold::new(threshold)?)))` to a request (e.g. `ItemReadOptions::with_operation_options`) or to the client defaults via `CosmosClientBuilder::with_operation_options`. The `AvailabilityStrategy`, `HedgingStrategy`, and `HedgeThreshold` types are now re-exported from `azure_data_cosmos`. When enabled, the driver speculatively dispatches the read to a second preferred region after the configured threshold elapses and returns whichever response classifies as final first, cancelling the losing leg structurally (no detached tasks); `AvailabilityStrategy::Disabled` turns hedging off for that scope, and when no strategy is configured the driver applies a built-in default for multi-region reads. ([#4432](https://github.com/Azure/azure-sdk-for-rust/pull/4432))

 - Added configurable retry limits for throttled (HTTP 429, rate-limited) requests, mirroring the .NET and Java SDKs' `ThrottlingRetryOptions`. A new nested `ThrottlingRetryOptions` group on `OperationOptions` (field `throttling_retry_options`) carries `max_retry_count` (env `AZURE_COSMOS_MAX_THROTTLE_RETRY_COUNT`, default `9`, `0` disables throttle retries) and `max_retry_wait_time` (default `30s`), settable per-request via `OperationOptions`/`OperationOptionsBuilder` and `ThrottlingRetryOptionsBuilder`. New client-wide setter `CosmosClientBuilder::with_throttling_retry_options(ThrottlingRetryOptions)` forwards the group as runtime-layer defaults. Both budgets apply *per transport-pipeline invocation*, not per logical operation — an operation that fans out across regions (failover, hedging) starts a fresh budget per leg; use `OperationOptions::end_to_end_latency_policy` to bound total per-operation wall-clock time. ([#4544](https://github.com/Azure/azure-sdk-for-rust/pull/4544))



Extra newline!

Fixed in 5bcd815. With the helper API removed this entry got rescoped too: the driver CHANGELOG now just records the Retry -> OperationRetry rename under Breaking Changes, and the stray blank line is gone.

analogrelay

My personal preference would be to remove all the helpers that dig through the requests and create new data (requested_regions(), hedging_started(), etc.) but I'm not going to block the PR on it :).

analogrelay · 2026-06-11T22:51:23Z

 - Added `PartitionKeyDefinition::with_kind(PartitionKeyKind)` and `PartitionKeyDefinition::with_version(PartitionKeyVersion)` consuming setters so callers can override the auto-inferred kind and the default `V2` version without resorting to private-field workarounds. `PartitionKeyDefinition::new(paths)` continues to auto-infer the kind from the path count.
 - Reshaped `PatchInstructions` API so that `PatchInstructions::new()` creates an empty instruction set, where individual operations can be added via `with_operation()`. The `PatchInstructions` type also implements `From<Vec<PatchOperation>>` for ergonomic construction from a vector.
 - Refactored the driver response surface: introduced `ResponseBody` (a `NoPayload` / `Bytes(Bytes)` / `Items(Vec<Bytes>)` enum with `single()`, `items()`, `into_single::<T>()`, `into_items::<T>()`, and `is_empty()` helpers), added typed `CosmosRequestHeaders` fields for query / changefeed headers (`max_item_count`, `incremental_feed`, `populate_index_metrics`, `populate_query_metrics`, `enable_cross_partition_query`) so callers no longer need raw `custom_headers`, the pipeline now auto-emits `x-ms-documentdb-isquery: True` and `Content-Type: application/query+json` for `OperationType::Query`, and `CosmosStatus` gained `PartialEq<StatusCode>`, `From<CosmosStatus> for StatusCode/u16`, and a `CosmosStatus::new(StatusCode)` constructor. ([#4401](https://github.com/Azure/azure-sdk-for-rust/pull/4401))
+


Extra newline

Fixed in 5bcd815 - removed the stray blank line. The hedging Features entry it followed was also removed (the SDK no longer adds any public surface here, so there is nothing to note in the SDK changelog).

analogrelay · 2026-06-11T22:51:54Z

+    ///
+    /// Renamed to [`ExecutionContext::OperationRetry`] to align with the
+    /// cross-SDK reason taxonomy. Retained for one release for source
+    /// compatibility; the serialized form also changes from `"retry"` to
+    /// `"operation_retry"`.
+    #[deprecated(since = "0.3.0", note = "use `ExecutionContext::OperationRetry`")]
    Retry,


Nah, just remove it. We'll have breaking changes pre-1.0, no need to deprecate and alias stuff

Done in 5bcd815 - removed Retry outright (no #[deprecated], no alias). It is renamed straight to OperationRetry, and the serialized form is now operation_retry. Agreed there is no point aliasing pre-1.0.

analogrelay · 2026-06-11T22:53:07Z

+/// wildcard arm.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
+#[non_exhaustive]
+pub enum RequestedRegionReason {


Seems like this could just be ExecutionContext rather than defining a separate enum. I've never been a huge fan of the ExecutionContext name anyway :). Maybe RequestPurpose? RequestIntent?

Agreed - removed the parallel RequestedRegionReason (and RequestedRegion) entirely in 5bcd815. ExecutionContext is now the single source of truth for a request's reason; callers read it directly off RequestDiagnostics::execution_context(), so there is no second enum to keep in sync. I left renaming ExecutionContext itself (-> RequestPurpose / RequestIntent) out of this PR to keep the blast radius small and called it out as future work in the spec - happy to do it here instead if you would prefer.

analogrelay · 2026-06-11T22:56:13Z

+    pub fn requested_regions(&self) -> Vec<RequestedRegion> {
+        self.requests
+            .iter()
+            .filter_map(|r| {
+                r.region().map(|region| RequestedRegion {
+                    region: region.clone(),
+                    reason: RequestedRegionReason::from(r.execution_context()),
+                })
+            })
+            .collect()
+    }


I'm not a huge fan of defining all these helpers before we know exactly what customers are going to do with them. In Rust, this causes allocations that a user may prefer to avoid. Of course, they can just iterate the requests themselves, but having our own helper here just feels unnecessary. IMO, just let the user dig into the requests for now. When we're generating diagnostic strings or possibly concise, loggable, descriptions of diagnostics output, it might make sense to calculate this, but for now I don't feel a need to add it. I'd rather be conservative with public API in this fresh v1.

Agreed, and thanks - removed requested_regions(), responded_regions(), and hedging_started() (plus the responded_with_service_reply predicate) in 5bcd815. Callers now dig into the already-public requests() / hedge_diagnostics() themselves and only allocate when they actually want a derived collection. The spec was rewritten to document the detection recipes instead of shipping helpers, with a note that we can add helpers later (e.g. when generating loggable diagnostic strings) once we know how customers use them.

Per reviewer feedback on PR Azure#4558, be conservative with the v1 public surface instead of shipping computed convenience helpers. - Remove DiagnosticsContext::requested_regions/responded_regions/ hedging_started and the RequestDiagnostics::responded_with_service_reply predicate. Callers read the already-public requests()/hedge_diagnostics() building blocks directly (allocating only when they need to). - Remove the parallel RequestedRegion / RequestedRegionReason types and the From<ExecutionContext> mapping; ExecutionContext is the single source of truth for a request's reason. Drop their driver and SDK re-exports. - Remove the deprecated ExecutionContext::Retry alias; pre-1.0 we rename outright to OperationRetry (serialized "retry" -> "operation_retry"). - Rewrite HEDGING_DETECTION_API_SPEC.md around the building-block approach, documenting detection recipes rather than helper methods. - Fix stray CHANGELOG newlines and rescope entries to the rename only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…s-hedging-pr-review

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

NaluTripician · 2026-06-15T22:15:21Z

@analogrelay thanks for the review. I took your personal preference and ran with it - rather than ship the convenience helpers, I removed them and reworked the PR around the alternate "let callers read the raw diagnostics" approach.

What changed (commit 5bcd815, plus a merge of main to clear the conflicts):

Removed DiagnosticsContext::requested_regions(), responded_regions(), and hedging_started(), the responded_with_service_reply predicate, and the parallel RequestedRegion / RequestedRegionReason types + From<ExecutionContext> mapping (and their driver/SDK re-exports). No new allocating helpers in this fresh v1.
Kept the Retry -> OperationRetry rename, but dropped the #[deprecated] alias per your other comment.
ExecutionContext is now the single source of truth for a request's reason - callers detect hedging by iterating the already-public requests() / hedge_diagnostics().
Rewrote HEDGING_DETECTION_API_SPEC.md to document the detection recipes against the existing building blocks instead of an API surface, and noted helpers can come later (e.g. when we build loggable diagnostic strings) once usage is clear.
Fixed the two stray CHANGELOG newlines and rescoped the entries.

Merge conflicts with main are resolved; build/clippy/fmt are clean and the driver lib suite (1808) + SDK lib tests pass. I also left ExecutionContext un-renamed for now (your RequestPurpose / RequestIntent idea) to keep the blast radius small - flagged as future work, but glad to fold it in here if you'd like.

NaluTripician · 2026-06-15T22:20:44Z

Note: holding this PR - please do not merge yet. I want to block it until my later diagnostics changes are checked in, so we can land them together and avoid churn on the diagnostics surface.

The two delimiter rows in the spec's tables used compact pipes (|---|---|---|) while their header and body rows are space-padded, tripping MD060/table-column-style and failing the CI Build Analyze stage. Pad the delimiter rows to match the surrounding style. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

NaluTripician · 2026-06-15T23:56:40Z

Pushed 8a0b110 to fix the failing Build Analyze stage: the two table delimiter rows in HEDGING_DETECTION_API_SPEC.md used compact pipes (|---|---|---|) while their header/body rows are space-padded, tripping MD060/table-column-style. Padded the delimiters to | --- | --- | --- |.

Verified locally: markdownlint-cli2 now reports 0 errors on the spec, and cspell is clean. The substantive review threads (drop the helper APIs, remove the parallel RequestedRegion/RequestedRegionReason enum, rename Retry -> OperationRetry without an alias, CHANGELOG newline) were already addressed in 5bcd815; the spec already documents the detection recipes against the raw diagnostics building blocks rather than shipping helpers.

Copilot AI review requested due to automatic review settings June 9, 2026 20:42

NaluTripician requested a review from a team as a code owner June 9, 2026 20:42

Copilot started reviewing on behalf of NaluTripician June 9, 2026 20:42 View session

github-actions Bot added the Cosmos The azure_cosmos crate label Jun 9, 2026

github-project-automation Bot added this to CosmosDB Rust SDK and Driver Jun 9, 2026

github-project-automation Bot moved this to Todo in CosmosDB Rust SDK and Driver Jun 9, 2026

NaluTripician force-pushed the nalutripician/hedging-spec-update branch from 023b20f to dcbddf2 Compare June 9, 2026 20:43

Copilot AI reviewed Jun 9, 2026

View reviewed changes

NaluTripician force-pushed the nalutripician/hedging-spec-update branch from 329e71b to 4e5049e Compare June 10, 2026 00:24

NaluTripician changed the title ~~[Cosmos][Spec] Refresh Hedging Detection API spec~~ [Cosmos] Refresh Hedging Detection API spec Jun 10, 2026

NaluTripician changed the title ~~[Cosmos] Refresh Hedging Detection API spec~~ [Cosmos] Refresh Hedging Detection API Jun 10, 2026

NaluTripician and others added 2 commits June 10, 2026 07:38

NaluTripician force-pushed the nalutripician/hedging-spec-update branch from 4e5049e to 1c295b3 Compare June 10, 2026 14:39

NaluTripician force-pushed the nalutripician/hedging-spec-update branch from 1c295b3 to 2c28269 Compare June 10, 2026 14:58

analogrelay reviewed Jun 11, 2026

View reviewed changes

analogrelay approved these changes Jun 11, 2026

View reviewed changes

NaluTripician and others added 3 commits June 15, 2026 15:08

Merge remote-tracking branch 'upstream/main' into nalutripician/cosmo…

aedc1f5

…s-hedging-pr-review

Merge main: drop duplicate ExecutionContext rename changelog entry

bef50d8

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

		- Exposed cross-regional read hedging. Enable it by attaching an `OperationOptions` built with `OperationOptionsBuilder::with_availability_strategy(AvailabilityStrategy::Hedging(HedgingStrategy::new(HedgeThreshold::new(threshold)?)))` to a request (e.g. `ItemReadOptions::with_operation_options`) or to the client defaults via `CosmosClientBuilder::with_operation_options`. The `AvailabilityStrategy`, `HedgingStrategy`, and `HedgeThreshold` types are now re-exported from `azure_data_cosmos`. When enabled, the driver speculatively dispatches the read to a second preferred region after the configured threshold elapses and returns whichever response classifies as final first, cancelling the losing leg structurally (no detached tasks); `AvailabilityStrategy::Disabled` turns hedging off for that scope, and when no strategy is configured the driver applies a built-in default for multi-region reads. ([#4432](https://github.com/Azure/azure-sdk-for-rust/pull/4432))

		- Added configurable retry limits for throttled (HTTP 429, rate-limited) requests, mirroring the .NET and Java SDKs' `ThrottlingRetryOptions`. A new nested `ThrottlingRetryOptions` group on `OperationOptions` (field `throttling_retry_options`) carries `max_retry_count` (env `AZURE_COSMOS_MAX_THROTTLE_RETRY_COUNT`, default `9`, `0` disables throttle retries) and `max_retry_wait_time` (default `30s`), settable per-request via `OperationOptions`/`OperationOptionsBuilder` and `ThrottlingRetryOptionsBuilder`. New client-wide setter `CosmosClientBuilder::with_throttling_retry_options(ThrottlingRetryOptions)` forwards the group as runtime-layer defaults. Both budgets apply per transport-pipeline invocation, not per logical operation — an operation that fans out across regions (failover, hedging) starts a fresh budget per leg; use `OperationOptions::end_to_end_latency_policy` to bound total per-operation wall-clock time. ([#4544](https://github.com/Azure/azure-sdk-for-rust/pull/4544))

Uh oh!

Conversation

NaluTripician commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What's implemented

Validation

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

analogrelay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NaluTripician commented Jun 15, 2026

Uh oh!

NaluTripician commented Jun 15, 2026

Uh oh!

NaluTripician commented Jun 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NaluTripician commented Jun 9, 2026 •

edited

Loading