Azure / Azure/azure-sdk-for-rust

Cosmos: Remove the PPAF-implies-PPCB enablement coupling now that PPCB is on by default

Open
#4,631 0 comments 1 reaction 1 assignee Assigned to @Meghana-Palaparthi View on GitHub
Cosmos
Dominant language
Rust
Stars
884
Forks
365
Avg merge
2d 19h
Merged PRs (30d)
109

Description

## Background

Historically, per-partition automatic failover (PPAF) being enabled by the
server account property `enable_per_partition_failover_behavior` was treated as
**implying** the per-partition circuit breaker (PPCB) should also be on. That
coupling predates PPCB being enabled by default.

After [#4588](https://github.com/Azure/azure-sdk-for-rust/pull/4588), PPCB
enablement is a driver-level `PartitionFailoverOptions` concern and PPCB now
defaults to **on** (`PartitionFailoverOptions::default().circuit_breaker_enabled
== true`). With PPCB on by default, deriving PPCB enablement from the PPAF
account property is redundant and makes the resolution logic harder to reason
about (PPCB enablement appears to depend on a PPAF signal that is really about a
different feature).

## Current behavior (the coupling to remove)

PPCB effective enablement is currently resolved as:

```
override.unwrap_or(account_property_ppaf || option_circuit_breaker_enabled)
```

i.e. the account's PPAF property OR-s into PPCB enablement. This lives at two
sites:

1. **`LocationStateStore` account-property refresh** —
`sdk/cosmos/azure_data_cosmos_driver/src/driver/routing/location_state_store.rs`
(the `sync_account_properties` / `apply_partition` path, ~lines 511-530):

```rust
next.per_partition_circuit_breaker_enabled = previous
.config
.circuit_breaker_enabled_override()
.unwrap_or_else(|| {
per_partition_automatic_failover_enabled // <-- PPAF account property
|| previous.config.circuit_breaker_enabled()
});
```

2. **`PartitionEndpointState::new`** —
`sdk/cosmos/azure_data_cosmos_driver/src/driver/routing/partition_endpoint_state.rs`
(~lines 66-72). At construction PPAF is always `false`, so this site is
effectively already `override.unwrap_or(circuit_breaker_enabled())`, but it
should be reviewed/aligned with site 1 for consistency.

## Proposed change

Drop the `per_partition_automatic_failover_enabled || ...` term from PPCB
resolution so PPCB enablement depends only on its own option (and the
authoritative env override). Target resolution:

```
per_partition_circuit_breaker_enabled =
circuit_breaker_enabled_override.unwrap_or(circuit_breaker_enabled_option)
```

The incident kill switch (`AZURE_COSMOS_PPCB_ENABLED_OVERRIDE`) must remain
authoritative over the base option (this is unchanged).

PPAF enablement itself (`per_partition_automatic_failover_enabled`) continues to
be driven by the account property — only its influence on **PPCB** is removed.

## Acceptance criteria

- [ ] PPCB effective enablement no longer reads
`enable_per_partition_failover_behavior`; it resolves from
`circuit_breaker_enabled` (option) plus the authoritative env override.
- [ ] PPAF enablement behavior is unchanged (still driven by the account
property).
- [ ] `AZURE_COSMOS_PPCB_ENABLED_OVERRIDE` remains authoritative over the base
option and the account property.
- [ ] Both resolution sites (`LocationStateStore` refresh and
`PartitionEndpointState::new`) are consistent.
- [ ] Update/extend the override-clearing comments in `LocationStateStore` that
currently reference "PPCB tracks PPAF here".
- [ ] Unit tests covering: PPCB on-by-default with PPAF account property off;
PPCB explicitly disabled via option while PPAF property is on (PPCB stays
off); override on/off still wins in both directions.
- [ ] CHANGELOG entry in `azure_data_cosmos_driver`.

## Notes / risk

- Behavior change: an account that previously turned PPCB **on** purely via the
PPAF account property (with the PPCB option explicitly set to `false`) will no
longer get PPCB. Given PPCB defaults to **on**, the only way to reach the new
"off" outcome is to explicitly disable the PPCB option, which is the intended
semantics. Call this out in the CHANGELOG.
- Keep the change scoped to enablement resolution; do not touch the
per-partition override-map clearing logic beyond comment updates.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.