Skip to content

fix(lifecycle): remove preStop primary demotion when failover disabled - #16

Merged
rikez merged 3 commits into
mainfrom
rikez/feat/no-prestop-failover-disabled
Oct 28, 2026
Merged

rikez merged 3 commits into
mainfrom
rikez/feat/no-prestop-failover-disabled

Conversation

@rikez

@rikez rikez commented Oct 15, 2026 •

Copy link
Copy Markdown
Collaborator

What

Removing the demoting preStop hook from the pod template doesn't help pods that already exist. Pod specs are immutable. Addresses the review on this PR: after an operator upgrade, a manual-failover cluster must still be writable with the primary on its pinned index.

  1. RollingUpdate + replication + automaticFailover: false now
    renders the StatefulSet as OnDelete, and the operator orders replacement: standbys first,
    pinned primary last.

  2. The pinned primary restarts in place. Before any deletion, the
    operator reads every pod's live Dolt role and defers the rollout while any role disagrees with
    status.currentPrimaryPodIndex.

  3. When no pod reports primary, the operator promotes the pod recorded in
    status.currentPrimaryPodIndex at epoch+1, and only when that pod
    holds a strictly highest epoch. Since only a demoting primary raises its own epoch (the hook is
    guarded by if [ "$ROLE" = "primary" ]), a unique maximum identifies the last primary, which
    demoted only after a standby confirmed caught up.

New status and events

  • Condition: PrimaryAvailable=true - a pod reports role primary
  • Condition: PrimaryAvailable=false - no safe primary could be proven
  • Event: `PrimaryReasserted - The recorded primary was restored
  • Event: NoPrimary - Reassert refused to promote.

Solves #15

@rikez
rikez requested a review from coffeegoddd October 15, 2026 19:05
@rikez rikez self-assigned this Oct 15, 2026

@coffeegoddd coffeegoddd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Handle existing pods when removing the preStop hook. This fixes newly created pods, but updating an existing StatefulSet triggers a rollout while its old primary still has the demotion hook. If that hook succeeds, the replacement starts in standby and automaticFailover: false prevents promotion, leaving no writable primary. Please cover the upgrade path and provide a safe migration/recovery mechanism.

Confirmed against b629c5f6c054 on minikube (Kubernetes 1.35.1, Dolt 2.4.1): the rollout completed with both pods Ready, both SQL roles remained standby for 30 seconds, and an insert failed with Database upgrade_probe is read-only. The old primary's shutdown log confirms a successful transition to standby.

Minimal minikube reproduction
  1. Start a disposable cluster:

    minikube start -p pr16-repro --driver=docker --cpus=2 --memory=6144 --kubernetes-version=v1.35.1

    Install the operator from base commit fd8a45a261dc using the normal CRDs/RBAC/deployment manifests, including the VolumeSnapshot CRD. Build an operator image from PR commit b629c5f6c054 and load it with minikube -p pr16-repro image load <pr-image> for the later upgrade.

  2. Create a two-pod DoltDB named upgrade, with its upgrade-credentials Secret (admin-user=admin, admin-password=repro-password) and these settings:

    spec:
      engineVersion: "2.4.1"
      replicas: 2
      updateStrategy: RollingUpdate
      replicationStrategy: DirectStandby
      replication:
        enabled: true
        primary: {podIndex: 0, automaticFailover: false}
      storage: {size: 1Gi, storageClassName: standard}
      resources:
        requests: {cpu: 250m, memory: 512Mi}
        limits: {memory: 1Gi}
      server:
        listener: {host: 0.0.0.0, port: 3306}
        cluster:
          remotesAPI: {port: 50051}
  3. Wait for readiness, then move the primary to ordinal 1 through a manual switchover (the operator bootstraps ordinal 0):

    kubectl --context pr16-repro wait --for=condition=Ready doltdb/upgrade --timeout=600s
    kubectl --context pr16-repro patch doltdb upgrade --type=merge \
      -p '{"spec":{"replication":{"primary":{"podIndex":1}}}}'
    kubectl --context pr16-repro wait \
      --for=jsonpath='{.status.currentPrimaryPodIndex}'=1 doltdb/upgrade --timeout=300s

    Create upgrade_probe.writes(id INT PRIMARY KEY) on upgrade-1, insert a row, and verify that row is readable on upgrade-0 before continuing.

  4. Upgrade only the operator image to the PR image. Wait for the StatefulSet template to lose preStop and for its rollout to finish. Query both pods:

    for i in 0 1; do
      kubectl --context pr16-repro exec upgrade-$i -c dolt -- \
        dolt --host 127.0.0.1 --no-tls -u admin -p repro-password sql -r csv \
        -q 'SELECT @@global.dolt_cluster_role, @@global.dolt_cluster_role_epoch'
    done

    Observed: upgrade-0 = standby,2, upgrade-1 = standby,3; an insert on upgrade-1 fails as read-only. Cleanup: minikube delete -p pr16-repro.

Ordinal 1 matters for this reproduction: native RollingUpdate replaces it before the healthy standby. With primary ordinal 0, I observed the old hook fail because the recently restarted standby was not fully caught up, so that run retained a writable primary.

@rikez

rikez commented Oct 27, 2026

Copy link
Copy Markdown
Collaborator Author

[P1] Handle existing pods when removing the preStop hook. This fixes newly created pods, but updating an existing StatefulSet triggers a rollout while its old primary still has the demotion hook. If that hook succeeds, the replacement starts in standby and automaticFailover: false prevents promotion, leaving no writable primary. Please cover the upgrade path and provide a safe migration/recovery mechanism.

Confirmed against b629c5f6c054 on minikube (Kubernetes 1.35.1, Dolt 2.4.1): the rollout completed with both pods Ready, both SQL roles remained standby for 30 seconds, and an insert failed with Database upgrade_probe is read-only. The old primary's shutdown log confirms a successful transition to standby.

Minimal minikube reproduction

@coffeegoddd Thanks for catching this case!

Here's the solution I came up with:

We can't stop the preStop hook from firing on pods that already exist, so instead of trying to avoid the demotion, we control when it happens and let the operator reassert the roles accordingly. This basically involves three changes:

  1. We take the rollout away from k8s. When replication is enabled and automaticFailover is false, the STS updateStrategy is rendered as onDelete, so k8s deletes nothing, and operator chooses the order: standbys first, pinned primary last.

  2. Keep the primary on the pinned index. Make sure the StatefulSet updates retains the pinned primary pod, matching status with Dolt pods.

  3. Reassert recorded primary, here's the logic:

  • Any unreadable pod → requeue
  • primaryCount == 1 → nothing to repair
  • primaryCount == 0 -> promote spec.replication.primary.podIndex.
  • else -> Mark as blocked NoPrimary
sequenceDiagram
    autonumber
    actor U as User
    participant K as Kubernetes
    participant S as Operator StatefulSet reconciler
    participant R as Operator Replication reconciler
    participant P0 as Pod 0 standby
    participant P1 as Pod 1 pinned primary wi

    U->>K: Upgrade the operator image

    Note over S,K: PART 1 - the operator takes the pod order
    S->>K: Patch pod template without preStop and strategy OnDelete
    Note right of K: One merge patch, so no window<br/>under a native rollout
    K-->>S: Template updated, no pods deleted

    Note over S,P1: PART 2 - keep the primary at the pinned index
    S->>P0: Read Dolt role and epoch
    P0-->>S: standby, epoch 2
    S->>P1: Read Dolt role and epoch
    P1-->>S: primary, epoch 2
    Note right of S: Roles agree with<br/>status.currentPrimaryPodIndex=1

    S->>K: Delete Pod 0, the standby first
    K->>P0: Stop and recreate without the hook
    P0-->>S: Ready, standby

    S->>P1: Check that no standby is behind
    P1-->>S: Pod 0 is caught up

    S->>K: Delete Pod 1 in place
    K->>P1: Stop Pod 1
    P1->>P1: Legacy hook runs, change role to standby, epoch 3
    K->>P1: Recreate without the hook
    P1-->>S: Ready, standby from disk
    Note over P0,P1: Both pods are standby, writes fail

    Note over R,P1: PART 3 - restore the prim
    R->>K: Confirm every pod is Ready
    R->>P0: Read role and epoch
    P0-->>R: standby, epoch 2
    R->>P1: Read role and epoch
    P1-->>R: standby, epoch 3

    alt Pod 1 holds the uniquely highest epoch
        Note right of R: Only a demoting primary raises<br/>its own epoch, so Pod 1 was<br/>provably the last primary
        R->>P1: DOLT_ASSUME_CLUSTER_ROLE prim
        P1-->>R: Accepted
        R->>K: Patch epoch 4 and PrimaryAvailable True
        R->>K: Event PrimaryReasserted
        P1->>P0: Replicate new writes
        Note over P0,P1: Pod 1 accepts writes
    else Proof fails
        R->>K: Patch PrimaryAvailable False, reason NoPrimary
        R->>K: Warning event with the cause
        Note over R: Replication repair and switchover still run,<br/>so changing podIndex remains a manual remedy
    end
Loading

@rikez
rikez requested a review from coffeegoddd October 27, 2026 20:46

@coffeegoddd coffeegoddd left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

rikez added 3 commits October 28, 2026 11:02
…er is disabled

- create integration test dedicated to scenarios for when failover is off.

Solves #15
The earlier operator gave each pod a preStop hook. This hook changes the role of a primary pod to standby. A pod specification is immutable. Therefore existing pods keep the hook after the upgrade. The solution has three parts. Part 1 and part 2 control the pod order. Part 3 restores the primary.

1. The dev upgrades the operator image. The cluster has two pods. Pod 1 is the pinned primary.
2. The operator removes the hook from the pod template.
3. The operator finds RollingUpdate, replication enabled, and automaticFailover: false.
4. The operator sets the StatefulSet strategy to OnDelete in the same write.
5. Kubernetes now deletes no pods. Kubernetes cannot stop the primary first.
6. The operator takes control of the pod order.
7. The operator reads the Dolt role of every pod. The operator compares each role with status.currentPrimaryPodIndex.
8. If one role disagrees, the operator stops the rollout. The operator tries again after 5 seconds.
9. The operator deletes pod 0 first. Pod 0 is a standby. Pod 0 restarts without the hook.
10. The operator makes sure that pod 0 is not behind pod 1.
11. The operator deletes pod 1 last. The operator restarts pod 1 in place. The operator starts no handover.
12. The hook in pod 1 runs one final time. The hook changes the role of pod 1 to standby. The epoch becomes 3.
13. Pod 1 restarts without the hook. Pod 1 reads the standby role from disk. Both pods are now standby.
14. The operator waits until both pods are ready. The operator then reads the role and the epoch of each pod.
15. The operator confirms five conditions. All pod states are readable. No pod reports primary. All other pods report standby. The recorded index is valid. The epoch of the recorded pod is the highest.
16. Pod 0 is standby,2. Pod 1 is standby,3. All five conditions pass.
17. The operator makes pod 1 the primary at epoch 4. The operator changes no other pod.
18. The operator writes the new epoch to the status. The operator sets PrimaryAvailable to True. The operator sends a PrimaryReasserted event.
19. Write operations on pod 1 are successful. Pod 0 receives the new data.
@rikez
rikez force-pushed the rikez/feat/no-prestop-failover-disabled branch from 02392fe to 8638651 Compare October 28, 2026 18:02
@rikez
rikez merged commit a576d3e into main Oct 28, 2026
12 of 13 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.

2 participants