Fix/synchronous standby name wildcard (#629)

* Clean-up what turns out to be unused code.

* Refrain from using synchronous_standby_names = '*'.

In case users would connect other nodes (not managed by pg_auto_failover)
then those would be the target of synchronous transactions, and that
wouldn't be expected.

Author: DimCitus

src/bin/pg_autoctl/cli_do_misc.c

@@ -79,50 +79,6 @@ keeper_cli_drop_replication_slot(int argc, char **argv)
 }
 
 
-/*
- * keeper_cli_enable_synchronous_replication implements the CLI to enable
- * synchronous replication on the primary.
- */
-void
-keeper_cli_enable_synchronous_replication(int argc, char **argv)
-{
-	KeeperConfig config = keeperOptions;
-	LocalPostgresServer postgres = { 0 };
-	bool missingPgdataOk = false;
-	bool pgNotRunningOk = false;
-
-	keeper_config_init(&config, missingPgdataOk, pgNotRunningOk);
-	local_postgres_init(&postgres, &(config.pgSetup));
-
-	if (!primary_enable_synchronous_replication(&postgres))
-	{
-		exit(EXIT_CODE_PGSQL);
-	}
-}
-
-
-/*
- * keeper_cli_disable_synchronous_replication implements the CLI to disable
- * synchronous replication on the primary.
- */
-void
-keeper_cli_disable_synchronous_replication(int argc, char **argv)
-{
-	KeeperConfig config = keeperOptions;
-	LocalPostgresServer postgres = { 0 };
-	bool missingPgdataOk = false;
-	bool pgNotRunningOk = false;
-
-	keeper_config_init(&config, missingPgdataOk, pgNotRunningOk);
-	local_postgres_init(&postgres, &(config.pgSetup));
-
-	if (!primary_disable_synchronous_replication(&postgres))
-	{
-		exit(EXIT_CODE_PGSQL);
-	}
-}
-
-
 /*
  * keeper_cli_add_defaults implements the CLI to add pg_auto_failover default
  * settings to postgresql.conf

src/bin/pg_autoctl/cli_do_root.c

@@ -57,32 +57,6 @@ CommandLine do_primary_adduser =
 					 "Create users on primary", NULL, NULL,
 					 NULL, do_primary_adduser_subcommands);
 
-CommandLine do_primary_syncrep_enable =
-	make_command("enable",
-				 "Enable synchronous replication on the primary server",
-				 "",
-				 "",
-				 NULL, keeper_cli_enable_synchronous_replication);
-
-CommandLine do_primary_syncrep_disable =
-	make_command("disable",
-				 "Disable synchronous replication on the primary server",
-				 "",
-				 "",
-				 NULL, keeper_cli_disable_synchronous_replication);
-
-CommandLine *do_primary_syncrep[] = {
-	&do_primary_syncrep_enable,
-	&do_primary_syncrep_disable,
-	NULL
-};
-
-CommandLine do_primary_syncrep_ =
-	make_command_set("syncrep",
-					 "Manage the synchronous replication setting on the primary server",
-					 NULL, NULL,
-					 NULL, do_primary_syncrep);
-
 CommandLine do_primary_slot_create =
 	make_command("create",
 				 "Create a replication slot on the primary server",
@@ -128,7 +102,6 @@ CommandLine do_primary_identify_system =
 
 CommandLine *do_primary[] = {
 	&do_primary_slot_,
-	&do_primary_syncrep_,
 	&do_primary_adduser,
 	&do_primary_defaults,
 	&do_primary_identify_system,

src/bin/pg_autoctl/pgsql.c

@@ -1616,22 +1616,6 @@ parseReplicationSlotMaintain(void *ctx, PGresult *result)
 }
 
 
-/*
- * pgsql_enable_synchronous_replication enables synchronous replication
- * in Postgres such that all writes block post-commit until they are
- * replicated.
- */
-bool
-pgsql_enable_synchronous_replication(PGSQL *pgsql)
-{
-	GUC setting = { "synchronous_standby_names", "'*'" };
-
-	log_info("Enabling synchronous replication");
-
-	return pgsql_alter_system_set(pgsql, setting);
-}
-
-
 /*
  * pgsql_disable_synchronous_replication disables synchronous replication
  * in Postgres such that writes do not block if there is no replica.

src/bin/pg_autoctl/pgsql.h

@@ -281,7 +281,6 @@ bool pgsql_replication_slot_create_and_drop(PGSQL *pgsql,
 											NodeAddressArray *nodeArray);
 bool pgsql_replication_slot_maintain(PGSQL *pgsql, NodeAddressArray *nodeArray);
 bool postgres_sprintf_replicationSlotName(int nodeId, char *slotName, int size);
-bool pgsql_enable_synchronous_replication(PGSQL *pgsql);
 bool pgsql_disable_synchronous_replication(PGSQL *pgsql);
 bool pgsql_set_default_transaction_mode_read_only(PGSQL *pgsql);
 bool pgsql_set_default_transaction_mode_read_write(PGSQL *pgsql);

src/bin/pg_autoctl/primary_standby.c

@@ -568,24 +568,6 @@ primary_set_synchronous_standby_names(LocalPostgresServer *postgres)
 }
 
 
-/*
- * primary_enable_synchronous_replication enables synchronous replication
- * on a primary postgres node.
- */
-bool
-primary_enable_synchronous_replication(LocalPostgresServer *postgres)
-{
-	PGSQL *pgsql = &(postgres->sqlClient);
-
-	log_trace("primary_enable_synchronous_replication");
-
-	bool result = pgsql_enable_synchronous_replication(pgsql);
-
-	pgsql_finish(pgsql);
-	return result;
-}
-
-
 /*
  * primary_disable_synchronous_replication disables synchronous replication
  * on a primary postgres node.

src/bin/pg_autoctl/primary_standby.h

@@ -76,7 +76,6 @@ bool postgres_replication_slot_create_and_drop(LocalPostgresServer *postgres,
 											   NodeAddressArray *nodeArray);
 bool postgres_replication_slot_maintain(LocalPostgresServer *postgres,
 										NodeAddressArray *nodeArray);
-bool primary_enable_synchronous_replication(LocalPostgresServer *postgres);
 bool primary_disable_synchronous_replication(LocalPostgresServer *postgres);
 bool postgres_add_default_settings(LocalPostgresServer *postgres);
 bool primary_create_user_with_hba(LocalPostgresServer *postgres, char *userName,

src/monitor/node_active_protocol.c

@@ -2192,7 +2192,13 @@ synchronous_standby_names(PG_FUNCTION_ARGS)
 			secondaryNode->goalState == REPLICATION_STATE_SECONDARY)
 		{
 			/* enable synchronous replication */
-			PG_RETURN_TEXT_P(cstring_to_text("*"));
+			StringInfo sbnames = makeStringInfo();
+
+			appendStringInfo(sbnames,
+							 "ANY 1 (pgautofailover_standby_%d)",
+							 secondaryNode->nodeId);
+
+			PG_RETURN_TEXT_P(cstring_to_text(sbnames->data));
 		}
 		else
 		{

tests/test_auth.py

@@ -61,14 +61,20 @@ def test_003_init_secondary():
     assert node2.wait_until_state(target_state="secondary")
     assert node1.wait_until_state(target_state="primary")
 
-    eq_(node1.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node1.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_2)",
+    )
 
 
 def test_004_failover():
     print()
     print("Calling pgautofailover.failover() on the monitor")
     cluster.monitor.failover(formation="auth")
     assert node2.wait_until_state(target_state="primary")
-    eq_(node2.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node2.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_1)",
+    )
 
     assert node1.wait_until_state(target_state="secondary")

tests/test_basic_operation.py

@@ -72,7 +72,10 @@ def test_004_init_secondary():
 
     assert node2.wait_until_state(target_state="secondary")
     assert node1.wait_until_state(target_state="primary")
-    eq_(node1.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node1.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_2)",
+    )
 
     assert node1.has_needed_replication_slots()
     assert node2.has_needed_replication_slots()
@@ -125,7 +128,9 @@ def test_007_005_disable_maintenance():
     assert node1.wait_until_state(target_state="secondary")
     assert node2.wait_until_state(target_state="primary")
 
-    node2.check_synchronous_standby_names(ssn="*")
+    node2.check_synchronous_standby_names(
+        ssn="ANY 1 (pgautofailover_standby_1)"
+    )
 
 
 def test_008_001_enable_maintenance_secondary():
@@ -146,7 +151,9 @@ def test_008_002_disable_maintenance_secondary():
     assert node1.wait_until_state(target_state="secondary")
     assert node2.wait_until_state(target_state="primary")
 
-    node2.check_synchronous_standby_names(ssn="*")
+    node2.check_synchronous_standby_names(
+        ssn="ANY 1 (pgautofailover_standby_1)"
+    )
 
 
 # the rest of the tests expect node1 to be primary, make it so
@@ -156,7 +163,10 @@ def test_009_failback():
     assert node2.wait_until_state(target_state="secondary")
     assert node1.wait_until_state(target_state="primary")
 
-    eq_(node1.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node1.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_2)",
+    )
 
 
 def test_010_fail_primary():
@@ -176,7 +186,10 @@ def test_012_start_node1_again():
     node1.run()
 
     assert node2.wait_until_state(target_state="primary")
-    eq_(node2.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node2.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_1)",
+    )
 
     assert node1.wait_until_state(target_state="secondary")
 
@@ -226,7 +239,10 @@ def test_019_run_secondary():
     assert node2.has_needed_replication_slots()
     assert node3.has_needed_replication_slots()
 
-    eq_(node2.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node2.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_3)",
+    )
 
 
 # In previous versions of pg_auto_failover we removed the replication slot
@@ -249,12 +265,18 @@ def test_020_multiple_manual_failover_verify_replication_slots():
     assert node2.has_needed_replication_slots()
     assert node3.has_needed_replication_slots()
 
-    eq_(node3.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node3.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_2)",
+    )
 
     print("Calling pg_autoctl perform promotion on node 2")
     node2.perform_promotion()
     assert node2.wait_until_state(target_state="primary")
-    eq_(node2.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node2.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_3)",
+    )
 
     assert node3.wait_until_state(target_state="secondary")
 
@@ -271,7 +293,10 @@ def test_020_multiple_manual_failover_verify_replication_slots():
 def test_021_ifdown_primary():
     print()
     assert node2.wait_until_state(target_state="primary")
-    eq_(node2.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node2.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_3)",
+    )
     node2.ifdown()
 
 
@@ -308,7 +333,10 @@ def test_023_ifup_old_primary():
     assert node2.wait_until_state("secondary")
     assert node3.wait_until_state("primary")
 
-    eq_(node3.get_synchronous_standby_names_local(), "*")
+    eq_(
+        node3.get_synchronous_standby_names_local(),
+        "ANY 1 (pgautofailover_standby_2)",
+    )
 
 
 def test_024_stop_postgres_monitor():

tests/test_multi_maintenance.py

@@ -51,7 +51,9 @@ def test_002_add_standby():
     # make sure we reached primary on node1 before next tests
     assert node1.wait_until_state(target_state="primary")
 
-    node1.check_synchronous_standby_names(ssn="*")
+    node1.check_synchronous_standby_names(
+        ssn="ANY 1 (pgautofailover_standby_2)"
+    )
 
 
 def test_003_add_standby():