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.

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

[WIP] Add command to restart all underlying pg #561

Conversation

prabhu43
Copy link
Contributor

@prabhu43 prabhu43 commented Sep 7, 2018

Fixes #88

This PR is to restart the underlying postgres instances of all keepers

Reason

Few parameters like max_connections and shared_buffers require restart of underlying postgres in each Keeper. We always had to run a pg_ctl stop -m fast -W on the node/pod to restart the postgres (keeper only does a reload)

Approach

Keeper

  • Stolon Keepers watch for a flag in ClusterData.Keeper.Status if schedulePgRestart is set to true
  • If true, a keeper would set it false and restart the underlying postgres using pgManager

CLI

  • stolonctl has a subcommand called pgrestart
  • It would set schedulePgRestart to true for all keepers (which would trigger a restart of all clusters)

Objective

Initially, we wanted to achieve a restart with 0 downtime by promoting an existing slave as a master as present in #88 However, it takes 7 seconds to elect a new master. Simply triggering a fast restart of postgres only cause 1 second downtime.

Please share your comments and feedbacks.

@sgotti
Copy link
Member

sgotti commented Sep 12, 2018

@prabhu43 Thanks for the PR, do you need a review or should I wait untile it's not a WIP and tests passes?

@prabhu43
Copy link
Contributor Author

@sgotti Please review the implementation and give your feedback. We marked it as WIP to write few more tests. We will fix the failing tests and write few more tests

@@ -779,10 +781,53 @@ func (p *PostgresKeeper) Start(ctx context.Context) {

case <-endUpdateKeeperInfo:
updateKeeperInfoTimerCh = time.NewTimer(p.sleepInterval).C
case <-restartPGTimerCh:
go func() {
p.restartPostgresIfScheduled(ctx)
Copy link
Member

Choose a reason for hiding this comment

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

I will prefer to not add a new time and a new function the read clusterdata again, this will also cause some race condition when an user asks to restart postgres while the main loop is acting on it. So I'll do this in the main check.

@dineshba
Copy link
Contributor

@prabhu43 can we close this

@prabhu43 prabhu43 closed this Oct 22, 2019
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.

stolonctl: implement postgres reload/restart
3 participants