Skip to content

fix: Wait for the cache to show a StatefulSet shrink before the next pass - #71

Merged
as51340 merged 2 commits into
mainfrom
fix/statefulset-shrink-expectations
Oct 9, 2026
Merged

as51340 merged 2 commits into
mainfrom
fix/statefulset-shrink-expectations

Conversation

@as51340

@as51340 as51340 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Problem

replicaCounts applies max(declared, cached spec.replicas). After shedRetiredPods lowers a StatefulSet, the next pass can still read the pre-shrink copy from the informer cache. The status write that ends the shedding pass requeues the cluster on the MemgraphCluster watch stream, which isn't ordered with the StatefulSet stream. That pass then:

  1. applies the old count again, so the StatefulSet controller may recreate the shed pod, and
  2. finds the member already unregistered or removed, and shrinks the StatefulSet a second time.

The end state is correct, so this is churn rather than data loss. But a retired coordinator or data instance pod can briefly come back on its retained volume.

Fix

This is the Kubernetes expectations pattern, shaped like ECK's ExpectedStatefulSetUpdates and narrowed to the one write whose stale read undoes it:

  • New internal/expectations: StatefulSets.Expect(name, uid, generation) and Satisfied(name, cached). An expectation is satisfied once the cached copy is gone, is a different object, or has reached the generation. It's forgotten once satisfied.
  • shedRetiredPods records the UID and generation its apply returned. To get them, apply now returns the written object, and appliedGenerations became appliedObjects.
  • replicaCounts checks each role's cached StatefulSet. If either is behind, the pass ends before applying anything and returns RequeueAfter: requeueWhilePending. The Owns StatefulSet event normally requeues it first.
  • The expectation is held in memory only. A restarted operator lists into a fresh cache that already holds the shrink, so only the process that wrote it can be ahead of its own cache.

Every other cached read stays as it is. The apply restates desired state, pod deletes carry a UID precondition, rollout's Stale check covers status, and membership is read live over Bolt. CLAUDE.md gets a convention bullet with this reasoning.

Tests

  • Unit tests in internal/expectations cover older and newer generations, a missing StatefulSet, a replaced UID, forgetting a satisfied expectation, and a later write replacing an earlier one.
  • A new envtest serves the pre-shrink StatefulSet through a laggingCache client, then checks that the StatefulSet's generation doesn't change and that the pass converges once the cache catches up. With the gate disabled the test fails: the generation goes 2 → 4, grown back and then shrunk again.
  • make test and make lint pass.

…pass

replicaCounts applies the larger of the declared count and the one the
cached StatefulSet carries. A pass that read the StatefulSet from before
shedRetiredPods lowered it applied the old count again, then shrank it a
second time once it found the member already gone; the StatefulSet
controller could recreate the shed pod in between. The MemgraphCluster
status write that ends the shedding pass requeues the cluster on a
separate watch stream, so that pass can start before the StatefulSet
event reaches the cache.

shedRetiredPods now records the UID and generation the apply returned in
an in-memory expectation (internal/expectations), and a pass whose cached
StatefulSet is the same object at a lower generation ends before applying
anything, until the StatefulSet event clears it. A restarted operator
lists into a fresh cache that already holds the shrink, so nothing is
persisted.
@as51340 as51340 self-assigned this Oct 9, 2026
kubectl wait fails at once on an object that does not exist yet, and the
operator creates the Deployment a moment after the patch, so the spec
raced it. It now polls the Available condition, like the init container
spec polls its pod.
@as51340
as51340 enabled auto-merge (squash) October 9, 2026 13:28
@as51340
as51340 disabled auto-merge October 9, 2026 13:28
@as51340
as51340 merged commit afc627e into main Oct 9, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant