Desultory refactoring of the code. (#745)

src/bin/pg_autoctl/cli_common.h

@@ -23,6 +23,7 @@ extern KeeperConfig keeperOptions;
 extern bool createAndRun;
 extern bool outputJSON;
 extern bool openAppHBAonLAN;
+extern bool dropAndDestroy;
 
 #define SSL_CA_FILE_FLAG 1      /* root public certificate */
 #define SSL_CRL_FILE_FLAG 2     /* certificates revocation list */
@@ -172,8 +173,15 @@ void cli_set_groupId(Monitor *monitor, KeeperConfig *kconfig);
 bool cli_create_config(Keeper *keeper);
 void cli_create_pg(Keeper *keeper);
 bool check_or_discover_hostname(KeeperConfig *config);
+
+int cli_drop_node_getopts(int argc, char **argv);
+void cli_drop_node(int argc, char **argv);
 void keeper_cli_destroy_node(int argc, char **argv);
 
+void cli_drop_node_from_monitor(KeeperConfig *config,
+								int64_t *nodeId, int *groupId);
+void cli_drop_local_node(KeeperConfig *config, bool dropAndDestroy);
+
 bool cli_getopt_ssl_flags(int ssl_flag, char *optarg, PostgresSetup *pgSetup);
 bool cli_getopt_accept_ssl_options(SSLCommandLineOptions newSSLOption,
 								   SSLCommandLineOptions currentSSLOptions);

src/bin/pg_autoctl/cli_do_monitor.c

@@ -352,7 +352,7 @@ cli_do_monitor_get_coordinator(int argc, char **argv)
 {
 	KeeperConfig config = keeperOptions;
 	Monitor monitor = { 0 };
-	NodeAddress coordinatorNode = { 0 };
+	CoordinatorNodeAddress coordinatorNode = { 0 };
 
 	bool missingPgdataIsOk = true;
 	bool pgIsNotRunningIsOk = true;
@@ -381,7 +381,7 @@ cli_do_monitor_get_coordinator(int argc, char **argv)
 		exit(EXIT_CODE_MONITOR);
 	}
 
-	if (IS_EMPTY_STRING_BUFFER(coordinatorNode.host))
+	if (IS_EMPTY_STRING_BUFFER(coordinatorNode.node.host))
 	{
 		fformat(stdout, "%s has no coordinator ready yet\n", config.formation);
 		exit(EXIT_CODE_QUIT);
@@ -395,8 +395,8 @@ cli_do_monitor_get_coordinator(int argc, char **argv)
 
 		json_object_set_string(root, "formation", config.formation);
 		json_object_set_number(root, "groupId", (double) config.groupId);
-		json_object_set_string(root, "host", coordinatorNode.host);
-		json_object_set_number(root, "port", (double) coordinatorNode.port);
+		json_object_set_string(root, "host", coordinatorNode.node.host);
+		json_object_set_number(root, "port", (double) coordinatorNode.node.port);
 
 		(void) cli_pprint_json(js);
 	}
@@ -405,8 +405,8 @@ cli_do_monitor_get_coordinator(int argc, char **argv)
 		fformat(stdout,
 				"%s %s:%d\n",
 				config.formation,
-				coordinatorNode.host,
-				coordinatorNode.port);
+				coordinatorNode.node.host,
+				coordinatorNode.node.port);
 	}
 }
 

src/bin/pg_autoctl/cli_drop_node.c

@@ -45,18 +45,11 @@
  * Global variables that we're going to use to "communicate" in between getopts
  * functions and their command implementation. We can't pass parameters around.
  */
-static bool dropAndDestroy = false;
+bool dropAndDestroy = false;
 static bool dropForce = false;
 
-static int cli_drop_node_getopts(int argc, char **argv);
-static void cli_drop_node(int argc, char **argv);
 static void cli_drop_monitor(int argc, char **argv);
 
-static void cli_drop_node_from_monitor(KeeperConfig *config,
-									   int64_t *nodeId,
-									   int *groupId);
-
-static void cli_drop_local_node(KeeperConfig *config, bool dropAndDestroy);
 static void cli_drop_local_monitor(MonitorConfig *mconfig, bool dropAndDestroy);
 
 static void cli_drop_node_with_monitor_disabled(KeeperConfig *config,
@@ -98,7 +91,7 @@ CommandLine drop_node_command =
  * cli_drop_node_getopts parses the command line options necessary to drop or
  * destroy a local pg_autoctl node.
  */
-static int
+int
 cli_drop_node_getopts(int argc, char **argv)
 {
 	KeeperConfig options = { 0 };
@@ -337,7 +330,7 @@ cli_drop_node_getopts(int argc, char **argv)
  * cli_drop_node removes the local PostgreSQL node from the pg_auto_failover
  * monitor, and when it's a worker, from the Citus coordinator too.
  */
-static void
+void
 cli_drop_node(int argc, char **argv)
 {
 	KeeperConfig config = keeperOptions;
@@ -504,7 +497,7 @@ cli_drop_monitor(int argc, char **argv)
  * for the given --hostname and --pgport, or from the given --formation and
  * --name.
  */
-static void
+void
 cli_drop_node_from_monitor(KeeperConfig *config, int64_t *nodeId, int *groupId)
 {
 	Monitor monitor = { 0 };
@@ -563,7 +556,7 @@ cli_drop_node_from_monitor(KeeperConfig *config, int64_t *nodeId, int *groupId)
  * cli_drop_local_node drops the local node files, maybe including the PGDATA
  * directory (when --destroy has been used).
  */
-static void
+void
 cli_drop_local_node(KeeperConfig *config, bool dropAndDestroy)
 {
 	Keeper keeper = { 0 };

src/bin/pg_autoctl/fsm.c

@@ -175,6 +175,9 @@
 #define COMMENT_INIT_TO_REPORT_LSN \
 	"Creating a new node from a standby node that is not a candidate."
 
+#define COMMENT_DROPPED_TO_REPORT_LSN \
+	"This node is being reinitialized after having been dropped"
+
 #define COMMENT_ANY_TO_DROPPED \
 	"This node is being dropped from the monitor"
 
@@ -200,7 +203,7 @@ KeeperFSMTransition KeeperFSM[] = {
 	 */
 	{ INIT_STATE, SINGLE_STATE, COMMENT_INIT_TO_SINGLE, &fsm_init_primary },
 	{ DROPPED_STATE, SINGLE_STATE, COMMENT_INIT_TO_SINGLE, &fsm_init_primary },
-
+	{ DROPPED_STATE, REPORT_LSN_STATE, COMMENT_DROPPED_TO_REPORT_LSN, &fsm_init_from_standby },
 
 	/*
 	 * The previous implementation has a transition from any state to the INIT
@@ -362,7 +365,7 @@ KeeperFSMTransition KeeperFSM[] = {
 	{ REPORT_LSN_STATE, PREP_PROMOTION_STATE, COMMENT_REPORT_LSN_TO_PREP_PROMOTION, &fsm_prepare_standby_for_promotion },
 
 	{ REPORT_LSN_STATE, FAST_FORWARD_STATE, COMMENT_REPORT_LSN_TO_FAST_FORWARD, &fsm_fast_forward },
-	{ FAST_FORWARD_STATE, PREP_PROMOTION_STATE, COMMENT_FAST_FORWARD_TO_PREP_PROMOTION, &fsm_cleanup_and_resume_as_primary },
+	{ FAST_FORWARD_STATE, PREP_PROMOTION_STATE, COMMENT_FAST_FORWARD_TO_PREP_PROMOTION, &fsm_cleanup_as_primary },
 
 	{ REPORT_LSN_STATE, JOIN_SECONDARY_STATE, COMMENT_REPORT_LSN_TO_JOIN_SECONDARY, &fsm_checkpoint_and_stop_postgres },
 	{ REPORT_LSN_STATE, SECONDARY_STATE, COMMENT_REPORT_LSN_TO_JOIN_SECONDARY, &fsm_follow_new_primary },
@@ -513,6 +516,7 @@ keeper_fsm_reach_assigned_state(Keeper *keeper)
 		{
 			bool ret = false;
 
+			/* avoid logging "#any state#" to the user */
 			if (transition.current != ANY_STATE)
 			{
 				log_info("FSM transition from \"%s\" to \"%s\"%s%s",
@@ -551,10 +555,19 @@ keeper_fsm_reach_assigned_state(Keeper *keeper)
 			}
 			else
 			{
-				log_error("Failed to transition from state \"%s\" "
-						  "to state \"%s\", see above.",
-						  NodeStateToString(transition.current),
-						  NodeStateToString(transition.assigned));
+				/* avoid logging "#any state#" to the user */
+				if (transition.current != ANY_STATE)
+				{
+					log_error("Failed to transition from state \"%s\" "
+							  "to state \"%s\", see above.",
+							  NodeStateToString(transition.current),
+							  NodeStateToString(transition.assigned));
+				}
+				else
+				{
+					log_error("Failed to transition to state \"%s\", see above.",
+							  NodeStateToString(transition.assigned));
+				}
 			}
 
 			return ret;

src/bin/pg_autoctl/fsm.h

@@ -67,7 +67,7 @@ bool fsm_report_lsn(Keeper *keeper);
 bool fsm_fast_forward(Keeper *keeper);
 bool fsm_prepare_cascade(Keeper *keeper);
 bool fsm_follow_new_primary(Keeper *keeper);
-bool fsm_cleanup_and_resume_as_primary(Keeper *keeper);
+bool fsm_cleanup_as_primary(Keeper *keeper);
 
 bool fsm_init_from_standby(Keeper *keeper);
 

src/bin/pg_autoctl/fsm_transition.c

@@ -1325,11 +1325,11 @@ fsm_fast_forward(Keeper *keeper)
 
 
 /*
- * fsm_cleanup_and_resume_as_primary cleans-up the replication setting and
- * start the local node as primary. It's called after a fast-forward operation.
+ * fsm_cleanup_as_primary cleans-up the replication setting. It's called after
+ * a fast-forward operation.
  */
 bool
-fsm_cleanup_and_resume_as_primary(Keeper *keeper)
+fsm_cleanup_as_primary(Keeper *keeper)
 {
 	LocalPostgresServer *postgres = &(keeper->postgres);
 
@@ -1340,13 +1340,6 @@ fsm_cleanup_and_resume_as_primary(Keeper *keeper)
 		return false;
 	}
 
-	if (!keeper_restart_postgres(keeper))
-	{
-		log_error("Failed to restart Postgres after changing its "
-				  "primary conninfo, see above for details");
-		return false;
-	}
-
 	return true;
 }