Hedge the Collection Read metadata call in Cosmos driver#4710
Draft
NaluTripician wants to merge 4 commits into
Draft
Hedge the Collection Read metadata call in Cosmos driver#4710NaluTripician wants to merge 4 commits into
NaluTripician wants to merge 4 commits into
Conversation
Extends cross-region read hedging to the Collection Read (container properties) metadata call by adding ResourceType::DocumentCollection to HEDGEABLE_RESOURCE_TYPES. This metadata read is on the critical path of nearly every operation (cold-start cache population and steady-state refresh), so hedging it trims the long tail when the primary region is briefly slow. Scoping is precise. Because HEDGEABLE_OPERATION_TYPES stays [Read], the only newly-eligible tuple is (DocumentCollection, Read) = the container point read. Container writes (Create/Delete) are excluded by the is_read_only() guard, and the container feed reads (read_all_containers -> ReadFeed, query_containers -> Query) are excluded because their operation types are not hedgeable. Crucially, user change-feed / cross-partition feed reads (Document + ReadFeed) remain unhedged -- widening operation types to ReadFeed (needed for the PartitionKeyRange routing-map read) would have caught those too, so that read is deliberately left to a follow-up that adds (resource, operation) tuple gating plus first-page-only continuation pinning. Adds should_hedge tests covering: container read eligible (multi-region) and ineligible (single region); read_all_containers, query_containers, and a user change-feed read all NOT hedged (the over-broadening guard). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Document the extension of cross-region read hedging to the Collection Read metadata call in both the driver (azure_data_cosmos_driver) and public (azure_data_cosmos) changelogs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Implements review feedback on the Collection Read hedging widen.
HEDGING_SPEC.md was still describing Phase 1 as {Document}-only, so the
doc had drifted from the code now that (DocumentCollection, Read) is
hedgeable. Updated the section 1 operation-type scope table, the section
5.1 phase-allowed ResourceType table, the section 15.1 unit-test table
(corrected should_hedge_non_document and listed the five new tests), and
the section 16 Phase 1/Phase 2 sections. Also resolved the spec's open
"metadata cache invalidation" question for the Container point read:
fastest-wins is safe because the driver only consumes the immutable _rid
and partition-key definition from that payload.
Added hedging_container_read_primary_slow, the metadata analogue of
hedging_read_primary_slow, which injects an 800ms delay on the East US
MetadataReadContainer and asserts the alternate region wins. This is
end-to-end proof through the pipeline that the HEDGEABLE_RESOURCE_TYPES
widen actually hedges the container-properties read, not just that
should_hedge returns true at the unit level.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The CI Analyze job runs cSpell over every file changed in the PR, which includes the driver CHANGELOG entry that describes container feed reads and writes as staying "unhedged". "unhedged" is a domain term (the sibling "hedgeable" is already ignored) but was missing from the ignore list, so the spell-check task failed. Added it to sdk/cosmos/.cspell.json ignoreWords in alphabetical order. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
|
@NaluTripician, is this superseded by #4608 ? |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Extends the driver's cross-region read hedging to the Collection Read metadata call (resolving container properties), by adding
ResourceType::DocumentCollectiontoHEDGEABLE_RESOURCE_TYPESindriver/pipeline/hedging_eligibility.rs.Container-properties resolution is on the critical path of nearly every operation — cold-start populates it and steady-state refreshes re-issue it. When hedging is enabled (the default for multi-region accounts), the driver now speculatively dispatches this read to a second preferred region after the threshold elapses, trimming the long tail when the primary region is briefly slow. It reuses the existing hedging machinery (threshold resolution, secondary-region selection, structural loser-cancellation,
HedgeDiagnostics) — no new API and no change to the enable/disable controls (AvailabilityStrategy/AZURE_COSMOS_HEDGING_ENABLED).Scoping (the important part)
The change is deliberately a one-constant widen with
HEDGEABLE_OPERATION_TYPESleft as[Read], so the only newly-eligible(resource, operation)tuple is(DocumentCollection, Read)— the container point read. Verified againstcosmos_operation.rs:read_container_by_name(Collection Read)(DocumentCollection, Read)create_container/delete_container(DocumentCollection, Create/Delete)is_read_only()guard)read_all_containers(DocumentCollection, ReadFeed)query_containers(DocumentCollection, Query)read_all_items)(Document, ReadFeed)The other critical-path metadata read — the PartitionKeyRange routing-map
ReadFeed— is intentionally left to a follow-up. Making it eligible requires addingReadFeedtoHEDGEABLE_OPERATION_TYPES, which (sinceDocumentis already hedgeable) would also make user change-feed / cross-partition feed reads eligible. Doing that correctly needs(resource, operation)tuple gating plus a first-page-only / winning-region continuation-pinning story for the paginated feed (the approach the .NET SDK's metadata-hedging change took). I kept this PR to the unambiguously-safe Collection Read so it's easy to review and can't over-broaden.Tests
Added
should_hedgeunit tests inhedging_eligibility.rs:read_all_containers,query_containers, and a user change-feed read (read_all_items) all not hedged — the over-broadening guard proving the widen didn't leak to feed reads.cargo test -p azure_data_cosmos_driver --lib→ 1929 passed (46 inhedging_eligibility, incl. the 5 new).cargo fmt --checkandcargo clippy --all-targetsclean.Context
The Rust SDK already shipped cross-region read hedging (#4432 and follow-ups) as a general framework;
hedging_eligibility.rsnotes metadata reads as a planned future phase. This PR delivers the Collection Read half of that. Ports the metadata-hedging intent from the .NET SDK (Azure/azure-cosmos-dotnet-v3#5999), adapted to this architecture and scoped down to the safe increment.