PodDisruptionBudget should be scoped per shard instead of per cluster

[erkules]

PodDisruptionBudget should be scoped per shard instead of per cluster

Problem

The operator currently creates a single PodDisruptionBudget for the entire ValkeyCluster with:

• maxUnavailable: 1
• a selector matching only the cluster label (valkey.io/cluster)

As a result, voluntary disruptions are limited across the entire cluster instead of being evaluated independently for each shard.

For example, with:

spec:
  shards: 3
  replicas: 2

the cluster consists of three independent shards:

Shard 0: Primary + Replica + Replica
Shard 1: Primary + Replica + Replica
Shard 2: Primary + Replica + Replica

A disruption affecting a pod in Shard 0 unnecessarily blocks a simultaneous disruption in Shard 2, even though the shards are independent failure domains.

Proposed solution

Instead of creating a single cluster-wide PodDisruptionBudget, create one PDB per shard.

Each PDB should select pods using both:

• valkey.io/cluster
• valkey.io/shard-index

For example:

selector:
  matchLabels:
    valkey.io/cluster: my-cluster
    valkey.io/shard-index: "0"

This results in one PDB for each shard.

PDB policy

I suggest using:

minAvailable: 1

for each shard.

The purpose of the PDB is to ensure that at least one node of each shard remains available during voluntary disruptions while allowing maintenance to proceed independently across different shards.

Benefits

• PDBs align with the shard-based architecture of Valkey Cluster.
• Independent shards no longer block each other's maintenance.
• Node drains and rolling updates can progress concurrently on different shards.
• Availability guarantees become scoped to the actual failure domain (the shard) instead of the entire cluster.

I'd be happy to contribute a PR implementing this change if the maintainers agree with the approach.

[jdheyburn]

Continuing the conversation from:

• fix(pdb): correct test assertions, pdbName call, and update docs #304 (comment)

The PDB provided was always meant to be a strict, minimal PDB so that we had something while we iterated on the design.

Availability guarantees become scoped to the actual failure domain (the shard) instead of the entire cluster.

If we were to change it to per-shard, what safeguards do we have to ensure that the cluster can still remain healthy if all primaries of a shard are being stopped at once? The primaries of the shard are used for failover coordination, so there is some dependency needed on ensuring we do not disrupt too many primaries.

I would avoid setting minAvailable: 1 for stateful workloads. For topologies with >1 Valkey replicas, it could take out all the replicas of a shard.

Perhaps we can iterate on a design that allows this to be flexible, as opposed to changing it for everyone.

spec:
  podDisruptionBudget:
    mode: Cluster|Shard|Disabled
    minAvailable:
    maxUnavailable:

[erkules]

Thanks, that helps clarify the motivation behind the current design.

My original concern was operational rather than functional. A single cluster-wide maxUnavailable: 1 serializes all voluntary disruptions across the cluster, even when they affect independent shards. That can make node drains and Kubernetes upgrades unnecessarily slow or even block them in some environments.

Your point about protecting failover coordination makes sense, though. If the real concern is avoiding disruption of too many current primaries at the same time, I'm wondering whether the protection should be tied to the current primary role rather than to all Valkey pods equally.

Of course, that would require the operator to know which pods are currently primaries, which is a more involved design. It sounds like this is really a broader discussion about what the PDB policy should express rather than simply changing it from cluster-wide to per-shard.

Beside that, I like this API. It cleanly separates the PDB scope from the disruption policy while preserving a conservative default. The only requirement I'd suggest is making minAvailable and maxUnavailable mutually exclusive, which is straightforward to enforce with CEL validation.

[jdheyburn]

If the real concern is avoiding disruption of too many current primaries at the same time, I'm wondering whether the protection should be tied to the current primary role rather than to all Valkey pods equally.

This is something else that has been considered too. Currently node-index=0 is the desired primary, although this can change during operations. We have an idea to introduce failbacks to node-index=0 to allow for topologies to be respected, but there is still a moment where the primary is node-index>0. We've thought about introducing a label similar to current-replication-role=primary, but we would need to ensure that is synced up with the actual state. Until that's implemented I don't believe it makes sense to introduce a PDB for limiting how many primaries can be disrupted at once.

[melancholictheory]

one framing that might help separate the concerns: the operator's own rolls don't go through the PDB, they're sequenced in the reconcile (proactive failover, one node at a time). the PDB only governs disruptions the operator doesn't drive, node drains, autoscaler scale-down, upgrades. so this whole discussion is really about how much an external drain is allowed to take at once, which is exactly erkules's slow-drain pain.

per-shard scoping is the right move for that, independent shards are independent failure domains and there's no reason a drain in shard 0 should block one in shard 2.

two things a per-shard PDB doesn't cover though, and i think they're what @jdheyburn is pointing at:

the cross-shard primary quorum. valkey needs a majority of primaries reachable to authorize any failover election. per-shard PDBs each allow one eviction independently, so a drain rolling across nodes can take out the primary in a majority of shards at the same time, and then no shard can complete a failover, the cluster wedges even though every individual PDB was satisfied. a per-shard PDB structurally can't express a cross-shard invariant. if that's the worry, it wants a second PDB selecting only current primaries with maxUnavailable = floor((shards-1)/2) so a majority always survives, and that needs the live current-primary label you mentioned. keyed on node-index=0 it protects the wrong pod after a failover.

minAvailable: 1 is also weaker than it reads. on a shard of primary + 2 replicas it allows two evictions at once, so a drain can take the primary and its in-sync replica together and leave nothing caught up to promote. maxUnavailable: 1 (or minAvailable = replicas) keeps it to one at a time, which is what actually preserves a promotable replica. worth keeping as the safe default even if the count becomes user-configurable per your API idea.

[jdheyburn]

Here is what I am thinking:

1. Change spec.podDisruptionBudget: Managed to be spec.podDisruptionBudget.mode: Cluster
2. Introduce a mechanism on labelling what is the current primary
3. Add support for per-shard PDBs (spec.podDisruptionBudget.mode: Shard), whereby:
   • each shard can have at most 1 pod unavailable at a time
   • no more than 1 primary can be disrupted at a time (perhaps even use floor((shards-1)/2))

[melancholictheory]

this lines up, glad the pieces fit. a few things on the primary side worth pinning down.

the formula degrades at small shard counts. floor((shards-1)/2) is 0 for 1 and 2 shards, so maxUnavailable: 0 on the primaries PDB, and no node hosting a primary can ever be drained. and it isn't just conservative: valkey can't auto-promote a replica without a majority of masters voting, so a 1 or 2 shard cluster has no working failover in the first place, PDB or not. i'd gate the primaries PDB to shards >= 3 and leave smaller clusters unprotected on that axis (with a note that draining a primary there is a manual, HA-less operation), rather than ship a config that silently blocks every primary drain.

between fixed "1 primary" and the formula: floor((shards-1)/2) is the one that preserves quorum while giving back the parallelism per-shard PDBs are for. fixed 1 keeps quorum too but serializes primary drains across the whole cluster, so a 9-shard cluster drains primaries one node at a time; the formula lets 4 go at once there and still keeps 5 masters voting. i'd go with the formula for >= 3.

on the overlap: a primary ends up covered by both its shard PDB and the primaries PDB. that's fine, the eviction API requires every covering PDB to allow an eviction, so they compose as an AND (a primary drains only if its shard can spare it and quorum still holds). worth being deliberate about it, since overlapping PDBs tend to raise eyebrows.

and the primary label is best-effort by nature: valkey runs its own election, so there's a window after a failover where the pod is the new primary but the operator hasn't relabelled yet, and the PDB guards the old one for that gap. the ValkeyNode controller already reads INFO for its role, so that's the natural place to stamp it, keeping the lag to one reconcile.

[jdheyburn]

I'll revisit those nuances when we come round to implementing 3.

and the primary label is best-effort by nature: valkey runs its own election, so there's a window after a failover where the pod is the new primary but the operator hasn't relabelled yet, and the PDB guards the old one for that gap. the ValkeyNode controller already reads INFO for its role, so that's the natural place to stamp it, keeping the lag to one reconcile.

For 2., I am going to implement #261 first, then raise another PR to hook into the same logic for updating the label.

Working on a PR for 1. now.

edit: PR for 1:

• refactor!: convert podDisruptionBudget to struct with mode field #308

[erkules]

I just wanted to say that I've really enjoyed this constructive discussion. I really appreciate how you took a simple proposal and evolved it into a much stronger design.
At least PDBs for Valkey Sentinel should be a lot more straightforward 💃

[melancholictheory]

likewise, this was a good one. and agreed on sentinel, the failover vote lives in the sentinel quorum rather than in the data masters, so the data pods don't carry the cross-shard coupling that made this one fiddly. the thing to watch there is protecting a majority of the sentinels themselves, same quorum idea, just moved to a different set of pods.