Skip to content

Fix cluster event processing - #3213

Open
jorsol wants to merge 5 commits into
zalando:masterfrom
jorsol:fix-cluster-event-processing
Open

jorsol wants to merge 5 commits into
zalando:masterfrom
jorsol:fix-cluster-event-processing

Conversation

@jorsol

@jorsol jorsol commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

When many clusters get updated at once (e.g. a change to every postgresql CR), the operator processes them far slower than its worker count allows, and single lost events can stall it for minutes. Measured on a kind cluster with 100 single-instance clusters, patching an env var into every CR took 32 minutes with the default settings, and 2 clusters ended up UpdateFailed although their pods rolled fine.

Four independent causes, one commit each:

  1. A busy worker blocks the events of all clusters. cache.FIFO.Pop holds the queue lock while the process function runs, and the worker processed the event inside it. queueClusterEvent therefore blocked until the worker finished, often a whole rolling update. Since it's called from the postgresql informer handler, the events of every cluster waited behind it. Workers sat idle (on average 2.6 of 8 busy), and the time between patching a CR and the operator queueing its event had a median of 953s. The periodic resync was blocked the same way: it queued only 15 SYNC events in 14 minutes.
  2. Pod events get lost. Pod events are sent to the waiting worker without blocking, over an unbuffered channel. When two events arrive back to back, the second is dropped, because the receiver hasn't re-entered select yet. A dropped DELETE event left the worker in waitForPodDeletion until pod_deletion_wait_timeout (10m) expired. Combined with (1), that froze the processing of all clusters for 10 minutes, twice in one run.
  3. Fixed 15s DB connect retry. The connect timeout was also used as the retry interval. Right after a pod restart the first attempt almost always fails, so most rolling updates waited a flat 15s before syncing roles.
  4. repair_period never ran on its own schedule. The resync ticker fired only every resync_period, when a sync was always due, so the repair branch was effectively dead. The last sync time was also saved after listing and queueing and compared in whole seconds, so the sync could slip by a tick.

Changes

  • process cluster events outside of the queue lock: Pop only takes the event off the queue, and processing happens after the lock is released.
    • Since events can now pile up in a queue, UPDATE events get a key per resourceVersion. Otherwise a later update would replace a pending one, and the first one's changes would be lost, because Cluster.Update works on the diff between old and new spec. SYNC/REPAIR/ADD events still replace each other.
    • A DELETE now discards pending events by cluster UID.
    • Also adds a mutex for the worker assignment map, which was accessed concurrently from the informer, the resync and the API server.
  • do not lose pod events while waiting on a pod:
    • The subscriber channel is buffered, and dropped events are logged.
    • The pod delete handler now handles informer tombstones.
    • An update that replaces the pod with a new instance (different UID) counts as a deletion.
    • After a switchover the operator now waits for the candidate's master label rather than any role label, since buffered events from before the promotion would otherwise satisfy the wait.
  • retry Postgres connections with exponential backoff: adds retryutil.RetryWithBackoff, which starts at 1s and doubles up to 15s, keeping the 2m overall timeout. This also resolves the existing TODO: make the retry exponential.
  • run cluster repair scans on the repair period:
    • The ticker runs at min(resync_period, repair_period), and each tick decides whether a sync or a repair is due.
    • The tick time is saved as the last sync/repair time.
    • A tolerance of a tenth of the tick period keeps early ticks from postponing a due run.
    • repair_period <= 0 disables repair scans.

Results

Single-node kind cluster, 12 CPUs. "Time to roll all clusters" = from patching an env var into every CR until all pods run the new spec.

100 single-instance clusters

master this PR
Defaults (8 workers, QPS 10): all rolled 1935s 315s
Defaults: time until the event is queued (p50 / p90) 953s / 1850s 0.1s / 0.1s
Defaults: workers busy (average) 2.6 of 8 7.2 of 8
Defaults: processing time per cluster (p50) 40s 22s
Defaults: clusters failed at the 10m timeout 2 0
64 workers, QPS 100: all rolled 91s 92s

At 64 workers the limit becomes the node itself (64 Spilo pods restarting at once on 12 CPUs). With this PR all 64 workers do work at the same time; on master the peak was 63.

Mixed: 33 single-instance + 22 three-instance clusters (99 pods), this PR

8 workers, QPS 10 64 workers, QPS 100
All rolled 287s 84s
All statefulsets patched 229s 11s
Write outage per 3-instance cluster (p50 / max) 2.0s / 2.5s 1.5s / 2.5s
Failed clusters, timeouts, dropped events 0 0

Write outages were measured with a probe inserting into every cluster's primary every 0.5s.

Behavior changes worth noting

  • Repair scans now actually run every repair_period (default 5m). Each one lists the CRs and queues a REPAIR event for clusters whose status isn't successful.
  • Multiple updates to the same CR that arrive while a worker is busy are processed one after another instead of possibly merged. This matches what effectively happened before, when queueing blocked.

Testing

  • New unit tests:
    • queue semantics: updates are not merged, and a delete discards pending events
    • buffered pod events, isPodGone, and the switchover waiting for the master label
    • RetryWithBackoffWorker
    • clusterListAndSync scheduling, including early ticks
  • The pod-event test fails with an unbuffered channel, and the early-tick cases fail without the tolerance.
  • go test -race ./pkg/... passes.
  • I ran the operator built from this branch on kind for all of the above, and checked the resync schedule live with resync_period=4m, repair_period=1m: repair scans ran every minute and a full sync every 4 minutes.

jorsol and others added 4 commits November 1, 2026 07:36
FIFO.Pop holds the queue lock while calling the process function. Processing
the cluster event inside of it blocked queueClusterEvent for this worker until
the event was done, e.g. for the whole rolling update of a cluster. Since the
postgresql informer handler queues the events, a single busy worker stalled
the events of all clusters, leaving the other workers idle. Pop the event and
process it after the lock is released.

Now that events can pile up in a queue, update events get a key per resource
version. Otherwise a pending update would be replaced by a later one and the
changes of the first one, based on the diff of old and new spec, get lost.
A delete event therefore discards pending events by cluster UID.

Also guard the worker assignment map, which is accessed from the informer
handlers, the periodic resync and the API server.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Pod events are sent to the subscriber of a pod without blocking. With an
unbuffered channel an event got dropped whenever the subscriber was not
waiting on the channel at that very moment, e.g. when two events arrive back
to back. A dropped delete event left the worker in waitForPodDeletion until
pod_deletion_wait_timeout (10m) expired and the update failed, although the
pod was recreated fine. Buffer the subscriber channel and log dropped events.

Also recognize a deletion the informer missed: handle tombstones in the pod
delete handler and treat an update replacing the pod with a new instance
(different UID) as deleted.

As events are buffered now, wait for the master label of the candidate after
a switchover instead of any role label, skipping earlier replica events.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The connection was retried every 15s (the connect timeout). Right after a pod
is (re)created the first attempt usually fails, so every rolling update spent
15s waiting before syncing roles. Start retrying after 1s and double the
interval up to 15s, keeping the overall retry timeout of 2m.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The resync ticker fired every resync period only, where a sync was always due,
so the repair scan of failed clusters effectively never ran every
repair_period. Tick at the shorter of both periods and decide whether a sync
or a repair is due.

Record the time of the tick as the last sync or repair time instead of the
time after listing and queueing all clusters, and tolerate ticks arriving
slightly early, so a due sync is not postponed by a whole tick. A repair
period that is not positive disables the repair scans.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@FxKu FxKu added the major Major feature changes or updates, e.g. feature rollout to a new country, new API calls. label Nov 10, 2026
@FxKu FxKu moved this to Waiting for review in Postgres Operator Nov 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major Major feature changes or updates, e.g. feature rollout to a new country, new API calls.

Projects

Status: Waiting for review

Development

Successfully merging this pull request may close these issues.

2 participants