Work around pg_replication_slot_advance xmin maintenance bug. (#815)

When the Postgres function pg_replication_slot_advance is called on a slot
that used to be active (using streaming replication) and is now
inactive (maintained manually) then the xmin of the slot is not maintained
anymore.

We believe this is a bug in Postgres, and it has been reported here:

  https://www.postgresql.org/message-id/flat/VI1PR83MB01897A49AE5C54A82F841D8999B09%40VI1PR83MB0189.EURPRD83.prod.outlook.com

To avoid situations where Postgres won't VACUUM dead rows and otherwise
maintains itself correctly, when a former primary node is available again
and joind a pg_auto_failover as a standby, we drop the pre-existing
replication slot and create new ones (where this time xmin is NULL).

Author: DimCitus

src/bin/pg_autoctl/fsm.c

@@ -381,8 +381,8 @@ KeeperFSMTransition KeeperFSM[] = {
 	 * When an old primary gets back online and reaches draining/draining, if a
 	 * failover is on-going then have it join the selection process.
 	 */
-	{ DRAINING_STATE, REPORT_LSN_STATE, COMMENT_DRAINING_TO_REPORT_LSN, &fsm_report_lsn },
-	{ DEMOTED_STATE, REPORT_LSN_STATE, COMMENT_DEMOTED_TO_REPORT_LSN, &fsm_report_lsn },
+	{ DRAINING_STATE, REPORT_LSN_STATE, COMMENT_DRAINING_TO_REPORT_LSN, &fsm_report_lsn_and_drop_replication_slots },
+	{ DEMOTED_STATE, REPORT_LSN_STATE, COMMENT_DEMOTED_TO_REPORT_LSN, &fsm_report_lsn_and_drop_replication_slots },
 
 	/*
 	 * When adding a new node and there is no primary, but there are existing

src/bin/pg_autoctl/fsm.h

@@ -64,6 +64,7 @@ bool fsm_start_maintenance_on_standby(Keeper *keeper);
 bool fsm_restart_standby(Keeper *keeper);
 
 bool fsm_report_lsn(Keeper *keeper);
+bool fsm_report_lsn_and_drop_replication_slots(Keeper *keeper);
 bool fsm_fast_forward(Keeper *keeper);
 bool fsm_prepare_cascade(Keeper *keeper);
 bool fsm_follow_new_primary(Keeper *keeper);

src/bin/pg_autoctl/fsm_transition.c

@@ -1025,6 +1025,24 @@ fsm_rewind_or_init(Keeper *keeper)
 		}
 	}
 
+	/*
+	 * This node is now demoted: it used to be a primary node, it's not
+	 * anymore. The replication slots that used to be maintained by the
+	 * streaming replication protocol are now going to be maintained "manually"
+	 * by pg_autoctl using pg_replication_slot_advance().
+	 *
+	 * There is a problem in pg_replication_slot_advance() in that it only
+	 * maintains the restart_lsn property of a replication slot, it does not
+	 * maintain the xmin of it. When re-using the pre-existing replication
+	 * slots, we want to have a NULL xmin, so we drop the slots, and then
+	 * create them again.
+	 */
+	if (!primary_drop_all_replication_slots(postgres))
+	{
+		/* errors have already been logged */
+		return false;
+	}
+
 	return true;
 }
 
@@ -1265,6 +1283,29 @@ fsm_report_lsn(Keeper *keeper)
 }
 
 
+/*
+ * fsm_report_lsn_and_drop_replication_slots is used when a former primary node
+ * has been demoted and gets back online during the secondary election.
+ *
+ * As Postgres pg_replication_slot_advance() function does not maintain the
+ * xmin property of the slot, we want to create new inactive slots now rather
+ * than continue using previously-active (streaming replication) slots.
+ */
+bool
+fsm_report_lsn_and_drop_replication_slots(Keeper *keeper)
+{
+	LocalPostgresServer *postgres = &(keeper->postgres);
+
+	if (!fsm_report_lsn(keeper))
+	{
+		/* errors have already been reported */
+		return false;
+	}
+
+	return primary_drop_all_replication_slots(postgres);
+}
+
+
 /*
  * When the selected failover candidate does not have the latest received WAL,
  * it fetches them from another standby, the first one with the most LSN

src/bin/pg_autoctl/primary_standby.c

@@ -511,6 +511,38 @@ primary_drop_replication_slot(LocalPostgresServer *postgres,
 }
 
 
+/*
+ * primary_drop_all_replication_slots drops all the replication slots found on
+ * a node.
+ *
+ * When a node has been demoted, the replication slots that used to be
+ * maintained by the streaming replication protocol are now going to be
+ * maintained "manually" by pg_autoctl using pg_replication_slot_advance().
+ *
+ * There is a problem in pg_replication_slot_advance() in that it only
+ * maintains the restart_lsn property of a replication slot, it does not
+ * maintain the xmin of it. When re-using the pre-existing replication slots,
+ * we want to have a NULL xmin, so we drop the slots, and then create them
+ * again.
+ */
+bool
+primary_drop_all_replication_slots(LocalPostgresServer *postgres)
+{
+	NodeAddressArray otherNodesArray = { 0 };
+
+	log_info("Dropping replication slots (to reset their xmin)");
+
+	if (!postgres_replication_slot_create_and_drop(postgres, &otherNodesArray))
+	{
+		log_error("Failed to drop replication slots on the local Postgres "
+				  "instance, see above for details");
+		return false;
+	}
+
+	return true;
+}
+
+
 /*
  * postgres_replication_slot_create_and_drop drops the replication slots that
  * belong to dropped nodes on a primary server, and creates replication slots

src/bin/pg_autoctl/primary_standby.h

@@ -71,6 +71,7 @@ bool primary_create_replication_slot(LocalPostgresServer *postgres,
 bool primary_drop_replication_slot(LocalPostgresServer *postgres,
 								   char *replicationSlotName);
 bool primary_drop_replication_slots(LocalPostgresServer *postgres);
+bool primary_drop_all_replication_slots(LocalPostgresServer *postgres);
 bool primary_set_synchronous_standby_names(LocalPostgresServer *postgres);
 bool postgres_replication_slot_create_and_drop(LocalPostgresServer *postgres,
 											   NodeAddressArray *nodeArray);