Unretried 409 conflict on Pod patch in ClusterStatefulSetWithPrimaryReconciliationHandler.fixPods during restarts
Split from #3215 (bonus issue), reported by @geass. Please refer to #3215 for the full original report.
Summary
ClusterStatefulSetWithPrimaryReconciliationHandler.fixPods patches Pods without the
retry-on-conflict wrapper used elsewhere in the operator, so a 409 during an active restart fails
the whole reconciliation cycle and is logged as an ERROR, instead of being retried.
Environment
- StackGres operator:
1.19.0(GA) - Seen consistently during
SGShardedDbOps/SGDbOpsrestart operations
Current behaviour
ERROR [io.st.op.conciliation] (SGCluster-ReconciliationLoop) Reconciliation of SGCluster
citus-qa.example-coord failed: io.fabric8.kubernetes.client.KubernetesClientException:
Failure executing: PATCH at: .../pods/example-coord-0?fieldManager=StackGres&force=true.
Message: Operation cannot be fulfilled on pods "example-coord-0": the object has been
modified; please apply your changes to the latest version and try again. ... reason=Conflict
...
at io.stackgres.operator.conciliation.cluster.ClusterStatefulSetWithPrimaryReconciliationHandler.lambda$fixPods$47(ClusterStatefulSetWithPrimaryReconciliationHandler.java:716)
at io.stackgres.operator.conciliation.cluster.ClusterStatefulSetWithPrimaryReconciliationHandler.fixPods(...)Self-healing (the next scheduled cycle succeeds), but noisy and inconsistent with the rest of the
codebase: WARN [io.st.co.RetryUtil] Will retry after 2 milliseconds due to error: ... 409 appears
for other call sites in the same logs.
Analysis
- The failing call is the unguarded patch at
stackgres-k8s/src/operator/src/main/java/io/stackgres/operator/conciliation/cluster/ClusterStatefulSetWithPrimaryReconciliationHandler.java:716(.forEach(pod -> handler.patch(context, pod, null))). - The operator already has the right helper:
KubernetesClientUtil.retryOnConflict(...)(stackgres-k8s/src/common/src/main/java/io/stackgres/common/kubernetesclient/KubernetesClientUtil.java:117-131), which retries with back-off on exactly this error (isConflictmatches 409 +"the object has been modified"), and is what produces theRetryUtilwarnings seen elsewhere. - Pods are the most contended objects during a restart (kubelet, Patroni, the StatefulSet controller and the operator all write to them), so this path is the one most likely to hit 409s.
Expected behaviour
The Pod patches performed by fixPods are wrapped in retryOnConflict (or the equivalent handler
level retry), so a conflict is retried with back-off and logged as a WARN retry like every other
conflict in the operator, instead of aborting the reconciliation cycle with an ERROR.
Note
While reviewing this, the sibling patch call sites in the same handler (fixNonDisruptablePods,
fixPodsPatroniLabels, fixPodsAnnotations, fixPodsOwnerReferences, fixPodsLabels and the
StatefulSet patches) should be checked for the same gap so the fix is not partial.