Model Cosmos region failover states - #5134
Conversation
Model region offline and online behavior, priority-driven failover phases, Strong multi-write gating, satellite write revocation, and hidden region buildout in the internal in-memory emulator. Add driver and public-client coverage for warm and cold topology changes, preference fallback, exclusions, session tokens, races, and replication safety. Document the behavior observed from live accounts and service code. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bda5b12-da90-43a1-afc4-5b6d3a9d2c14
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Remove a public documentation link to a private configuration method so the Analyze job can build workspace documentation with warnings denied. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bda5b12-da90-43a1-afc4-5b6d3a9d2c14
Allow the service-specific topology terms used by the in-memory emulator so CI cSpell validation accepts the documented offline and buildout scenarios. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bda5b12-da90-43a1-afc4-5b6d3a9d2c14
|
Azure Pipelines: Successfully started running 1 pipeline(s). 3 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Models Cosmos region lifecycle, failover transitions, write gating, and replication-safe enrollment in the in-memory emulator.
Changes:
- Adds offline/online, failover-priority, and transition-state modeling.
- Adds hidden region enrollment and pending replication replay.
- Documents and tests routing and recovery behavior across driver and public SDK layers.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
sdk/cosmos/.cspell.json |
Adds topology terminology. |
sdk/cosmos/azure_data_cosmos/docs/in-memory-emulator-spec.md |
Documents new region semantics. |
sdk/cosmos/azure_data_cosmos/tests/in_memory_emulator_tests/mod.rs |
Registers public SDK tests. |
sdk/cosmos/azure_data_cosmos/tests/in_memory_emulator_tests/topology_parity.rs |
Tests public client topology behavior. |
sdk/cosmos/azure_data_cosmos_driver/src/in_memory_emulator/client.rs |
Models offline DNS failures. |
sdk/cosmos/azure_data_cosmos_driver/src/in_memory_emulator/config.rs |
Implements topology states and transitions. |
sdk/cosmos/azure_data_cosmos_driver/src/in_memory_emulator/store.rs |
Exposes lifecycle APIs and enrollment replay. |
sdk/cosmos/azure_data_cosmos_driver/src/in_memory_emulator/system_properties.rs |
Emits filtered account locations. |
sdk/cosmos/azure_data_cosmos_driver/tests/in_memory_emulator_tests/mod.rs |
Registers driver tests. |
sdk/cosmos/azure_data_cosmos_driver/tests/in_memory_emulator_tests/region_online_offline.rs |
Covers lifecycle, failover, and routing behavior. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Keep offline-triggered failover priorities aligned with service behavior, preserve retired region identity on rejected re-adds, and correct test docs. Make delayed region enrollment replay mutation-safe through catch-up, including deletes, concurrent registration, finalization, and region removal. Add regression coverage for each corrected edge case. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9bda5b12-da90-43a1-afc4-5b6d3a9d2c14
Review summaryI found four correctness cases that look worth fixing before merge:
I added inline comments with the concrete sequences and possible fixes. |
| // are atomic relative to each other: a mutation is either in | ||
| // this initial snapshot or appended to the new journal. | ||
| let in_flight = self.in_flight_replications.lock().unwrap(); | ||
| let pending: Vec<_> = in_flight.values().cloned().collect(); |
There was a problem hiding this comment.
🔴 Blocking · Correctness: Stale Mutation Replay
Could this replay mutations in the wrong order when a region is added?
For example, if a create is still pending but a newer delete has already completed, the delete is removed from this map while the create remains. The new region is seeded without the item, then the old create is replayed and brings it back.
I reproduced this by delaying a create, completing the delete, and removing/re-adding the target region. Could we track the latest mutation or tombstone per item instead of replaying every operation that happens to still be in flight? Should the delayed task also verify that it is still targeting the same region incarnation?
There was a problem hiding this comment.
will fix this and the following in a follow up pr. Thanks for hte feedback
| ); | ||
| tokio::spawn(async move { | ||
| tokio::time::sleep(delay).await; | ||
| target.catch_up_from(&seed_source); |
There was a problem hiding this comment.
🔴 Blocking · Correctness: Write Loss
Could delayed catch-up overwrite a write that has already committed locally?
A delayed region is already advertised and can accept writes. A write can be committed locally and pause before replicate() adds it to the catch-up journal. If this snapshot runs in between, it overwrites that local write. The journal does not contain the write yet, and when the request resumes, normal replication skips the source region, so the write is never restored there.
Could we make the local commit and journal registration atomic relative to catch-up finalization, or register the mutation before applying it locally?
| .filter(|r| { | ||
| r.name() == self.write_region | ||
| || self.failing_over_from.as_deref() == Some(r.name()) | ||
| || self.next_write_region.as_deref() == Some(r.name()) |
There was a problem hiding this comment.
🔴 Blocking · Correctness: Stale Failover State
Should changing write mode clear any active failover state?
If we announce a single-write failover, switch to multi-write, and then switch back to single-write, the old next_write_region becomes visible again. I reproduced this sequence and the final writable list contained both East and West instead of only East.
Could we either reject write-mode changes while a failover is active or clear next_write_region and failing_over_from when the mode changes?
| // failover: surviving regions move up and the offlined former hub | ||
| // moves to the lowest-priority position. | ||
| topology.priority_order.retain(|name| name != region_name); | ||
| topology.priority_order.push(region_name.to_string()); |
There was a problem hiding this comment.
🔴 Blocking · Correctness: Priority Invariant
Could this leave priority_order inconsistent when an earlier candidate is already offline?
For example, starting with [East, West, Central], if West is offline and East is then offlined, Central is correctly promoted, but the stored order becomes [West, Central, East]. That leaves offline West at position zero even though Central is the write region. I reproduced this with a focused test.
Could we move the promoted region to position zero before moving the former write region to the end?
Summary
NextWriteRegion/PreviousWriteRegiontransition phases