Added option to create streaming standby cluster. #804

ermajn · 2020-01-28T11:30:54Z

After this, there are two options of standby cluster: streaming_host, and s3_wal.
In case of streaming_host option, standby will connect to primary (specified in manifest) and do clone based on basebackup. For that a small refernce to basebackup script in spilo needs to be fixed (zalando/spilo#404).
Also, secret with credentials for standby user are handled manualy in this case. I'm not sure about implementation under #507, but this is minimalistic implementation that basically passes required ENVs to spilo, and it actualy works.

senthilcodr · 2020-02-24T09:51:07Z

Hi all,
This PR has been open for a while now. Can the reviewers please provide their feedback?

senthilcodr · 2020-04-03T04:06:12Z

@ermajn, does this PR support promoting of the standby cluster using the operator? (Similar to this: patroni/patroni#1096)

If not are you planning to add the same?

FxKu · 2020-04-03T10:24:46Z

Thanks @ermajn for this PR and sorry it took so long to give first feedback. I see one problem with the new required standby_method field. This would break all clusters already running as a standby. We should choose to ignore one standby setup if all fields are present, similar to what we do in the clone section where we clone from S3 when uid and timestamp are present. Otherwise, a basebackup is done from the specified cluster.

I tend to ignore S3 when streaming host and port are set. If it's incomplete take s3_wal_path. In the CRD validation no field should be set to required then. I think this way, everybody who already uses the standby feature can continue as is.

Three more minor requests:

Could you reflect your changes in the user docs, too?
Could you add unit tests similar to the one for cloning env?
Rebase with master :)

senthilcodr · 2020-06-24T07:10:08Z

@ermajn, is this PR still active? If not, I can raise a new one based on these changes.

ermajn · 2020-06-24T07:17:13Z

@senthilcodr yes its active, let me finish all requested task...

machine424 · 2021-03-27T21:33:13Z

Hi,
We are really interested in this feature, could it be ready for 1.7?

FxKu · 2021-03-29T07:24:56Z

Sure. Unfortunately, my main complaint from last summer is still not addressed: Backwards compatibility. The standby_method field is a breaking change. What to do with running standby clusters? I think we could do it similar to what we have with cloning. If e.g. host is present choose this method, else S3 like before.

ermajn · 2021-03-29T07:28:57Z

Hello, yes requirement is still there. It would be great if we can include it.
@FxKu if your propose about backward compatibility is acceptible, I can put some effort in implementing it that way.

…it will fall back to s3_wal

ermajn · 2021-11-01T09:16:34Z

@FxKu this one is old but it seems to be required. I have added now backward compatibility, it falls back to "s3_wal" when standby_method is not specified. Is there anything more needed in order to merge this PR?

FxKu · 2022-03-02T10:33:01Z

@ermajn I resolved the conflicts. I'm still not a fan of the standby_method. Why not use a streaming cluster if host is present and use WAL when it's not. We do something similar with cloning. If there's a timestamp specified we clone from basebackup, if not we clone from WAL. This would make things easier for users and devs. The behavior has to be documented, of course. At the moment only the reference docs were adjusted.

At first, I wasn't sure about the standby_secret_name either, but I see that with that you don't need to copy values from the secret of the source cluster into the replication user's secret of the standby cluster, right?

FxKu · 2022-03-02T10:34:06Z

pkg/cluster/k8sres.go

+		c.logger.Infof(msg, description.StandbyHost)
+
+		result = append(result, v1.EnvVar{Name: "STANDBY_HOST", Value: description.StandbyHost})
+		result = append(result, v1.EnvVar{Name: "STANDBY_PORT", Value: description.StandbyPort})


I think STANDBY_PORT can be omitted when it's not set. Patroni will default to 5432.

FxKu · 2022-03-02T10:39:29Z

pkg/cluster/k8sres.go

+	if description.StandbyMethod == "s3_wal" || description.StandbyMethod == "gs_wal" {
+  	if description.S3WalPath != "" {
+  		// standby with S3, find out the bucket to setup standby
+	  	msg := "Standby from S3 bucket using custom parsed S3WalPath from the manifest %s "
+		  c.logger.Infof(msg, description.S3WalPath)
+
+	  	result = append(result, v1.EnvVar{
+	  		Name:  "STANDBY_WALE_S3_PREFIX",
+		  	Value: description.S3WalPath,
+		  })
+  	} else if description.GSWalPath != "" {
+	  	msg := "Standby from GS bucket using custom parsed GSWalPath from the manifest %s "
+	  	c.logger.Infof(msg, description.GSWalPath)
+
+	  	envs := []v1.EnvVar{
+	  		{
+	  			Name:  "STANDBY_WALE_GS_PREFIX",
+	  			Value: description.GSWalPath,
+	  		},
+		  	{
+			  	Name:  "STANDBY_GOOGLE_APPLICATION_CREDENTIALS",
+				  Value: c.OpConfig.GCPCredentials,
+			  },
+		  }
+		  result = append(result, envs...)
+  	}
+		result = append(result, v1.EnvVar{Name: "STANDBY_METHOD", Value: "STANDBY_WITH_WALE"})
+		result = append(result, v1.EnvVar{Name: "STANDBY_WAL_BUCKET_SCOPE_PREFIX", Value: ""})


sorry, my conflict resolving messed up indentation in this part

FxKu · 2022-03-02T10:40:42Z

docs/reference/cluster_manifest.md

+* **standby_secret_name**
+  Specify secret name that is used to look-up for a password that
+  is need to connect to primary host. Password should be under `password` key
+  of a secret. This is *required* be specified when using `streaming_host` standby method.


It is not required so define it, since you default to the old behavior in the code.

FxKu · 2022-03-02T10:41:26Z

manifests/standby-manifest.yaml

-    s3_wal_path: "s3://path/to/bucket/containing/wal/of/source/cluster/"
+    s3_wal_path: "s3://path/to/bucket/containing/wal/of/source/cluster/" # Make this a standby cluster and provide the s3 bucket path of source cluster for continuous streaming. 
+    standby_method: "s3_wal" # s3_wal or streaming_host; 
+    # standby_host: ""


would be nice to provide a reasonable example here? Same for the user docs.

FxKu · 2022-04-04T13:44:30Z

I will close this PR now since we merged #1830. Still, thanks @ermajn for bringing it up. Sorry for the feedback being too sporadic.

Added option to create streaming standby cluster.

cc0e8d8

ermajn requested review from CyberDem0n, FxKu, Jan-M, RafiaSabih, avaczi, erthalion and sdudoladov as code owners January 28, 2020 11:30

FxKu added this to the 1.5 milestone Feb 25, 2020

FxKu added the spilo Issue more related to Spilo label Feb 25, 2020

Merge remote-tracking branch 'upsteram/master'

e4aa8cd

FxKu modified the milestones: 1.5, 1.6 Apr 23, 2020

Merge branch 'master' of https://github.com/zalando/postgres-operator

92d6358

FxKu removed this from the 1.6 milestone Aug 26, 2020

ermajn added 7 commits January 15, 2021 10:20

Merge branch 'master' of https://github.com/zalando/postgres-operator

a86e748

Merge branch 'master' of github.com:ermajn/postgres-operator

fbf43b2

Forgot during a merge.

127cbcf

We want to allow standby cluster connecting to remote master

1cc7166

Merge branch 'master' of https://github.com/zalando/postgres-operator

7c9cdf3

Merge branch 'master' of https://github.com/zalando/postgres-operator

eff8f8d

Changed validation crds so it represents new standby field values.

b6ce92e

machine424 mentioned this pull request Jun 30, 2021

add the possibility to create a standby cluster that streams from a remote primary #1544

Closed

ermajn added 3 commits October 29, 2021 09:11

Merge branch 'zalando:master' into master

fdffd5f

Implemented backward compatibility, if standby_method not specified, …

2d6598f

…it will fall back to s3_wal

Removed unnesesary comment, clean-up

a5bcc5b

FxKu added this to the 1.8 milestone Nov 1, 2021

FxKu mentioned this pull request Feb 18, 2022

Replication in standby mode #1782

Closed

Merge branch 'master' into master

91194fa

FxKu reviewed Mar 2, 2022

View reviewed changes

FxKu closed this Apr 4, 2022

Added option to create streaming standby cluster. #804

Added option to create streaming standby cluster. #804

Uh oh!

Conversation

ermajn commented Jan 28, 2020

Uh oh!

senthilcodr commented Feb 24, 2020

Uh oh!

senthilcodr commented Apr 3, 2020

Uh oh!

FxKu commented Apr 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

senthilcodr commented Jun 24, 2020

Uh oh!

ermajn commented Jun 24, 2020

Uh oh!

machine424 commented Mar 27, 2021

Uh oh!

FxKu commented Mar 29, 2021

Uh oh!

ermajn commented Mar 29, 2021

Uh oh!

ermajn commented Nov 1, 2021

Uh oh!

FxKu commented Mar 2, 2022

Uh oh!

FxKu Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

FxKu Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

FxKu Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

FxKu Mar 2, 2022

Choose a reason for hiding this comment

Uh oh!

FxKu commented Apr 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

FxKu commented Apr 3, 2020 •

edited

Loading