Skip to content
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

fix: replica cluster should restart after promotion #4399

Merged
merged 4 commits into from
May 22, 2024
Merged

fix: replica cluster should restart after promotion #4399

merged 4 commits into from
May 22, 2024

Conversation

litaocdl
Copy link
Collaborator

@litaocdl litaocdl commented Apr 29, 2024 •
edited by mnencia

When a replica cluster is promoted, the archive_mode is changed from
always to on. This change requires a restart because Postgres
does not reload the configuration during the promotion.

Closes: #4172

@github-actions github-actions bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.21 release-1.22 release-1.23 labels Apr 29, 2024
@github-actions GitHub Actions
Copy link
Contributor

❗ By default, the pull request is configured to backport to all release branches.

  • To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
  • To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

@litaocdl litaocdl requested a review from a team as a code owner May 8, 2024 09:58
@jsilvela
Copy link
Collaborator

jsilvela commented May 9, 2024

/test limit=local

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented May 9, 2024

@jsilvela, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/9014692079

jsilvela
jsilvela approved these changes May 9, 2024
View reviewed changes
internal/management/controller/instance_controller.go Outdated
@@ -1114,7 +1113,7 @@ func (r *InstanceReconciler) reconcilePrimary(ctx context.Context, cluster *apiv
if err := r.handlePromotion(ctx, cluster); err != nil {
return false, err
}
restarted = true
promoted = true
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not your PR but the existing code.
This is asymmetric. We decide that the instance is a primary with IsPrimary, which checks for the existence of "recovery.conf"

Let's say we:

  1. successfully promote the instace
  2. when trying to update the Status or drop the connections, we fail

Then the next time around we'll see the instance is a primary, and we won't retry.

Copy link
Member

Choose a reason for hiding this comment

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

If we don't update the status, the Status patch step is retried.
The drop connection is not vital. It is there only to prevent a long-running session through the read-only service from silently becoming a read-write one.

internal/management/controller/instance_controller.go Outdated Show resolved Hide resolved
@litaocdl
Copy link
Collaborator Author

update the pr by removing the unused variable

@fcanovai
Copy link
Contributor

/test level=4

@github-actions GitHub Actions
Copy link
Contributor

@fcanovai, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/9173885815

@github-actions github-actions bot added the ok to merge 👌 This PR can be merged label May 21, 2024
@litaocdl @mnencia
fix: replica cluster should restart after promote
d7d835c 
Signed-off-by: Tao Li <tao.li@enterprisedb.com>
jsilvela and others added 3 commits May 22, 2024 17:30
@jsilvela @mnencia
Update internal/management/controller/instance_controller.go
dbd6507 
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
@litaocdl @mnencia
remove unused variable
8dd23ca 
Signed-off-by: Tao Li <tao.li@enterprisedb.com>
@litaocdl @jsilvela @mnencia
Update internal/management/controller/instance_controller.go
94865a1 
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Signed-off-by: Tao Li <tao.li@enterprisedb.com>
@mnencia mnencia changed the title fix: replica cluster should restart after promote fix: replica cluster should restart after promotion May 22, 2024
@mnencia mnencia merged commit 33d7b65 into main May 22, 2024
30 checks passed
@mnencia mnencia deleted the dev/cnp-4172 branch May 22, 2024 16:16
cnpg-bot pushed a commit that referenced this pull request May 22, 2024
@litaocdl
fix: replica cluster should restart after promotion (#4399)
f775bf3 
When a replica cluster is promoted, the `archive_mode` is changed from
`always` to `on`. This change requires a restart because Postgres
does not reload the configuration during the promotion.

Closes: #4172

Signed-off-by: Tao Li <tao.li@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
(cherry picked from commit 33d7b65)
cnpg-bot pushed a commit that referenced this pull request May 22, 2024
@litaocdl
fix: replica cluster should restart after promotion (#4399)
ea34c7a 
When a replica cluster is promoted, the `archive_mode` is changed from
`always` to `on`. This change requires a restart because Postgres
does not reload the configuration during the promotion.

Closes: #4172

Signed-off-by: Tao Li <tao.li@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
(cherry picked from commit 33d7b65)
cnpg-bot pushed a commit that referenced this pull request May 22, 2024
@litaocdl
fix: replica cluster should restart after promotion (#4399)
4cce211 
When a replica cluster is promoted, the `archive_mode` is changed from
`always` to `on`. This change requires a restart because Postgres
does not reload the configuration during the promotion.

Closes: #4172

Signed-off-by: Tao Li <tao.li@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
(cherry picked from commit 33d7b65)
dougkirkley pushed a commit to dougkirkley/cloudnative-pg that referenced this pull request Jun 11, 2024
@litaocdl @jsilvela
fix: replica cluster should restart after promotion (cloudnative-pg#4399
5d96557 
)

When a replica cluster is promoted, the `archive_mode` is changed from
`always` to `on`. This change requires a restart because Postgres
does not reload the configuration during the promotion.

Closes: cloudnative-pg#4172

Signed-off-by: Tao Li <tao.li@enterprisedb.com>
Signed-off-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Co-authored-by: Jaime Silvela <jaime.silvela@enterprisedb.com>
Signed-off-by: Douglass Kirkley <dkirkley@eitccorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mnencia mnencia mnencia approved these changes

@jsilvela jsilvela jsilvela approved these changes

@fcanovai fcanovai fcanovai approved these changes

@gbartolini gbartolini Awaiting requested review from gbartolini

@leonardoce leonardoce Awaiting requested review from leonardoce

@phisco phisco Awaiting requested review from phisco

@sxd sxd Awaiting requested review from sxd

@armru armru Awaiting requested review from armru

Assignees
No one assigned
Labels
backport-requested ◀️ This pull request should be backported to all supported releases ok to merge 👌 This PR can be merged release-1.21 release-1.22 release-1.23
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: Replica cluster require a restart after promote
4 participants
@litaocdl @jsilvela @fcanovai @mnencia