Skip to content

Re-create pods only if all replicas are running #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 16 commits into from
Apr 20, 2020
Merged

Re-create pods only if all replicas are running #903

merged 16 commits into from
Apr 20, 2020

Conversation

sdudoladov
Copy link
Member

@sdudoladov sdudoladov commented Apr 7, 2020 •
edited
Loading

Operator currently kills pods too eagerly, for example, when re-init is still running.

Here a sequence of events leading to the problem:

  1. Re-init starts on the replica
  2. Rolling update comes
  3. Operator kills a replica pod being re-initialized almost immediately because it selects for recreation all pods belonging to a PG cluster (called from here) and later skips only the master

Re-doing re-init on the same pod later suffers from the same problem because the rolling upgrade may not complete. For instance, when a replica on pod -0 is being constantly killed but pod-1 is the yet-to-be-updated master.

The current workaround for this problem is to manually shut down the operator, let the re-init complete and then start the operator again.

The PR fixes this by asking Patroni to confirm there is no PG replicas in in the creating-replica state before deleting any pod. If that check fails, pod re-creation is postponed until next Sync .

@FxKu can you please link related issues ?

@sdudoladov sdudoladov added this to the 1.5 milestone Apr 7, 2020
@sdudoladov
Copy link
Member Author

As of commit 84dc06bfb755482c41ee1ca096f52436b92e0ac7, this patch prevents incorrect termination of replicas in the following scenario (tested manually for now):

  1. Get some data to make re-init long enough
    pgbench -i -s 1000 -n &> pgbench.log # approx. 17 GB
  2. start re-init on the replica
    patronictl reinit $SCOPE replica-name
  3. Cause the rolling upgrade, for example by updating resources

The patched operator under such circumstances will report update success without actually re-creating the pods. The 1st Sync after reinit terminates will find out incomplete rolling update thanks to annotations and complete it.

Note Patroni returns the 503 code for GET on replica_ip:8008, so presence of

 http get response: &{Status:503 Service Unavailable StatusCode:503 Proto:HTTP/1.0

in the logs is not an error in this case.

@sdudoladov sdudoladov changed the title [WIP] Re-create pods only if all replicas are running Re-create pods only if all replicas are running Apr 9, 2020
@FxKu
Copy link
Member

FxKu commented Apr 20, 2020

👍

1 similar comment
@sdudoladov
Copy link
Member Author

👍

@sdudoladov sdudoladov merged commit 3c91bde into master Apr 20, 2020
@FxKu FxKu mentioned this pull request Apr 20, 2020
Closed
@FxKu FxKu mentioned this pull request May 4, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avaczi avaczi Awaiting requested review from avaczi

@CyberDem0n CyberDem0n Awaiting requested review from CyberDem0n

@erthalion erthalion Awaiting requested review from erthalion

@FxKu FxKu Awaiting requested review from FxKu FxKu is a code owner

@Jan-M Jan-M Awaiting requested review from Jan-M Jan-M is a code owner

@RafiaSabih RafiaSabih Awaiting requested review from RafiaSabih

Assignees

@sdudoladov sdudoladov

@FxKu FxKu

Labels
bug zalando
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.
2 participants
@sdudoladov @FxKu