Updated synchronous_standby_names to be correct

[gclough]

Fixes #269

Updated synchronous_standby_names to be correct, even when {{ postgresql_synchronous_standby_num_sync }} isn't specified.

https://www.postgresql.org/docs/9.6/static/runtime-config-replication.html

This parameter specifies a list of standby servers using either of the following syntaxes:

num_sync ( standby_name [, ...] )
standby_name [, ...]

[gclough]

How do I resubmit this to Travis... as it seems to have failed on their end?

[gclough]

@WolfgangPecho , can you verify this update is correct for v10. I'm pretty sure it is, as I've test it and it matches the doco.

[gclough]

This is the new v10 syntax.

https://www.postgresql.org/docs/10/static/runtime-config-replication.html#GUC-SYNCHRONOUS-STANDBY-NAMES

[gclough]

This is the v9.6 syntax... which is different from anything <= 9.5, but is also different from v10. It's unique, and only for this one version of PostgreSQL.

https://www.postgresql.org/docs/9.6/static/runtime-config-replication.html#GUC-SYNCHRONOUS-STANDBY-NAMES

[gclough]

New parameters required for v9.6, and v10

[WolfgangPecho]

@gclough I am a bit busy at work and a bit late here ;-).
I have fetched your commit 1e5f77b #300 and ran tests/playbook.yml with vagrant for

• centos-7
• centos-6
• ubuntu-16.04
• ubuntu-14.04

All passed.
For posgresql 10. And debian-9 passed too.

[gclough]

That's great, thanks @WolfgangPecho . Hopefully this will be verified and merged soon, as the sync standby handling is one of the good features in v9.6 and v10.

[gclough]

I've done some testing, to merge in the "quoted" hostnames, as raised in #337:

Using PostgreSQL v9.3, and two sync standby's:
--extra-vars='{"postgresql_version":9.3, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":["server1","server2"]}'

Gives:
synchronous_standby_names = '"server1","server2"' # standby servers that provide sync rep

v9.3, No sync:
--extra-vars='{"postgresql_version":9.3, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":[]}'

Gives:

synchronous_standby_names = '' # standby servers that provide sync rep

v9.6, two sync:
--extra-vars='{"postgresql_version":9.6, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":["server1","server2"]}'

Gives:
synchronous_standby_names = '1 ("server1","server2")' # standby servers that provide sync rep

v9.6, no sync:
--extra-vars='{"postgresql_version":9.6, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":[]}'

Gives:
synchronous_standby_names = '' # standby servers that provide sync rep

v10, two sync:
--extra-vars='{"postgresql_version":10, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":["server1","server2"]}'

Gives:
synchronous_standby_names = 'ANY 1 ("server1","server2")' # standby servers that provide sync rep

v10, No sync:
--extra-vars='{"postgresql_version":10, "postgresql_synchronous_standby_choose_sync":"ANY", "postgresql_synchronous_standby_num_sync": 1, "postgresql_synchronous_standby_names":[]}'

Gives:
synchronous_standby_names = '' # standby servers that provide sync rep

[gclough]

@jlozadad ... I merged in a fix from #338 by @hakoerber ... but I'm not sure how to give attribution? If you could help I'd appreciate it.

[aoyawale]

no idea either. you can remove the commit if you want.

[hakoerber]

It's fine. Just wanted to tell you that I think unusual to incorporate fixes from other people's merge requests into your commits.

Anyway, the merge request looks good to me apart from two small issues:

We need documentation about postgresql_synchronous_standby_num_sync and postgresql_synchronous_standby_choose_sync, because their behaviour changed. I order to retain the same functionality, I had to change

postgresql_synchronous_standby_num_sync: "ANY 1"

to

postgresql_synchronous_standby_num_sync: "1"
postgresql_synchronous_standby_choose_sync: "ANY"

I like the second way much better, it just has to be documented properly I think.

Secondly, the diff is now quite huge due to the merge commit. When I do a

git diff upstream/master gclough/sync_standby_v9.6_syntax

I get a much nicer patch. Maybe it is a good idea to make completely new commits with this patch and commit them?

[gclough]

Secondly, the diff is now quite huge due to the merge commit. When I do a
git diff upstream/master gclough/sync_standby_v9.6_syntax
I get a much nicer patch. Maybe it is a good idea to make completely new commits with this patch and commit them?

Good idea. I’m still quite new to GIT and have a little trouble squashing commits. Would you like to do that, and commit it back? That way it will retain the credit that we both worked on this, right?

I see what you're saying about the backwards compatibility... but as the v10 merge was only pushed 14 days ago, is it a big issue? Would you prefer to see it documented in the defaults/main.yml, or somewhere else?

[gclough]

@hakoerber ... I'm not sure why, but my local "master" is a bit goofy. I've got a lot of PR's in progress, otherwise I'd blow it away and start again. :o(

I think I've pared it down to a minimal commit, and whenever I try to squash those commits into one then it fails the auto-merge. If this isn't right, then please squash them and re-commit it under your account, as this will tag your username in this PR too, right?

[UnderGreen]

@gclough is the PR ready for merge?

[gclough]

Yes, I believe so. This allows the v10 syntax to be used without a kludge.

Is there any way to attribute @hakoerber too, as he contributed to this via #338?