restart instances via rest api instead of recreating pods, fixes bug with being unable to decrease some values, like max_connections

[yanchenko-igor]

This PR is intended to solve an issue with values like max_connections being decreased, at the moment after the pod restarting the new decreased value doesn't apply unless instance is restarted by api. When we compare statefulsets we ignore bootstrap.dcs section so pod will not restart after changing parameters which matters only on cluster bootstrap.

[yanchenko-igor]

Can this PR be reviewed? if there other way to fix the bug let's discuss it.

[sdudoladov]

hello @yanchenko-igor

I will have a look today

[Jan-M]

i wonder if this list should not be in the config map, this could be the hard coded default. Spilo image may change and not sure this always warrants a new operator build

[yanchenko-igor]

I am not sure I understand what you mean, this list is what @CyberDem0n gave me when we discussed how it should work.

[sdudoladov]

@yanchenko-igor could you share a link to the discussion ?

[yanchenko-igor]

@sdudoladov it was private discussion in Slack.

[Jan-M]

Thanks for chipping in on this important issue.

For such a change to rolling upgrades maybe worth having an end 2 end test added or confirm there is one checking respective changes.

[sdudoladov]

Please also comment in the admin docs on when restarts should happen instead of pod re-creation

[sdudoladov]

@yanchenko-igor could you share a link to the discussion ?

[CyberDem0n]

Let's say that we changed work_mem from '4MB' to '16MB'.
Patroni will update postgresql.conf and do pg_ctl reload.
That doesn't require a restart.

[yanchenko-igor]

How we can distinguish parameters which require restart from those which don't?

[CyberDem0n]

You have to wait for a while (ttl seconds should be enough), and call Patroni API on every pod:

$ curl -s http://localhost:8008/patroni | jq .
{
  "pending_restart": true,

If there is pending_restart flag is set - postgres in this pod must be restarted.

Also, the /restart endpoint expects to get the json object, where you can specify {"restart_pending":true}, so Patroni will restart postgres only if it is required.

Waiting for ttl seconds is mandatory.

[yanchenko-igor]

Added {"restart_pending":true} and waiting after restart.

[yanchenko-igor]

Hello, I need help with e2e tests, I don't understand why they fail, can someone support me? They seem to be unrelated to my changes.

[Jan-M]

Hi! Right now you should not spend time on the e2e test, we are aware of this not being as reliable as we want it to be and also the feedback time seems really slow.

We are looking into this in a separate PR, lets continue this once we have this sorted out.

[FxKu]

Did some local tests and it works. Had to ensure that other custom changes so the config are not overwritten by the restart. However, I noticed that a restart happens on each sync if parameters are defined. I think, checkAndSetGlobalPostgreSQLConfiguration should read the config and compare with existing parameters in the manifest and only add them to optionsToSet if they differ.

[FxKu]

@yanchenko-igor with pending_restart an actual restart will not happen on each sync, but still seeing the messages in the logs and events makes it quite confusing:

Events:
  Type    Reason  Age                  From               Message
  ----    ------  ----                 ----               -------
  Normal  Update  119s (x79 over 44h)  postgres-operator  restarting Postgres server within pods
  Normal  Update  87s (x79 over 44h)   postgres-operator  Postgres server restart done - all instances have been restarted

I think, it's better if we read the config from patroni and compare it with what's in the manifest under postgresql.parameters.

[FxKu]

@yanchenko-igor Before you start working on it again, maybe we will merge the PR now as is, because I have to build up on it anyway and could then refactor that part.

[FxKu]

👍

[sdudoladov]

👍

[FxKu]

Thanks again @yanchenko-igor for this important contribution and sorry it took so long to merge. Thanks for your patience and responsiveness.