Implement support for citus.local_hostname. (#731)

* Implement support for citus.local_hostname.

* Allow retry when configuration file does not exists.

There seem to be a race condition at process startup where we overwrite the
configuration file when merging the command line arguments with the
pre-existing setup. For a short while the configuration file is reported
missing (ENOENT).

Implement a very small retry loop so that `pg_autoctl do pgsetup wait` is
more reliable, in particular in the test suite.

Author: DimCitus

src/bin/pg_autoctl/cli_do_misc.c

@@ -88,13 +88,22 @@ keeper_cli_add_default_settings(int argc, char **argv)
 {
 	KeeperConfig config = keeperOptions;
 	LocalPostgresServer postgres = { 0 };
-	bool missingPgdataOk = false;
-	bool postgresNotRunningOk = false;
 
-	keeper_config_init(&config, missingPgdataOk, postgresNotRunningOk);
+	bool missingPgdataIsOk = true;
+	bool pgIsNotRunningIsOk = true;
+	bool monitorDisabledIsOk = true;
+
+	if (!keeper_config_read_file(&config,
+								 missingPgdataIsOk,
+								 pgIsNotRunningIsOk,
+								 monitorDisabledIsOk))
+	{
+		exit(EXIT_CODE_BAD_CONFIG);
+	}
+
 	local_postgres_init(&postgres, &(config.pgSetup));
 
-	if (!postgres_add_default_settings(&postgres))
+	if (!postgres_add_default_settings(&postgres, config.hostname))
 	{
 		log_fatal("Failed to add the default settings for streaming replication "
 				  "used by pg_auto_failover to postgresql.conf, "

src/bin/pg_autoctl/config.c

@@ -286,10 +286,35 @@ ProbeConfigurationFileRole(const char *filename)
 
 	log_debug("Probing configuration file \"%s\"", filename);
 
-	if (!read_ini_file(filename, configOptions))
+	/*
+	 * There is a race condition at process startup where a configuration file
+	 * can disappear while being overwritten. Reduce the chances of that
+	 * happening by making more than one attempt at reading the file.
+	 */
+	char *fileContents = NULL;
+
+	for (int attempts = 0; fileContents == NULL && attempts < 3; attempts++)
 	{
-		log_fatal("Failed to parse configuration file \"%s\"", filename);
-		exit(EXIT_CODE_BAD_CONFIG);
+		long fileSize = 0L;
+
+		if (read_file_if_exists(filename, &fileContents, &fileSize))
+		{
+			break;
+		}
+
+		pg_usleep(100 * 1000);  /* 100ms */
+	}
+
+	if (fileContents == NULL)
+	{
+		log_error("Failed to read configuration file \"%s\"", filename);
+		return PG_AUTOCTL_ROLE_UNKNOWN;
+	}
+
+	if (!parse_ini_buffer(filename, fileContents, configOptions))
+	{
+		log_error("Failed to parse configuration file \"%s\"", filename);
+		return PG_AUTOCTL_ROLE_UNKNOWN;
 	}
 
 	log_debug("ProbeConfigurationFileRole: %s", config.role);

src/bin/pg_autoctl/fsm_transition.c

@@ -232,7 +232,7 @@ fsm_init_primary(Keeper *keeper)
 		return false;
 	}
 
-	if (!postgres_add_default_settings(postgres))
+	if (!postgres_add_default_settings(postgres, config->hostname))
 	{
 		log_error("Failed to initialize postgres as primary because "
 				  "adding default settings failed, see above for details");

src/bin/pg_autoctl/ini_file.c

@@ -26,14 +26,27 @@ read_ini_file(const char *filename, IniOption *optionList)
 {
 	char *fileContents = NULL;
 	long fileSize = 0L;
-	IniOption *option;
 
 	/* read the current postgresql.conf contents */
 	if (!read_file(filename, &fileContents, &fileSize))
 	{
 		return false;
 	}
 
+	return parse_ini_buffer(filename, fileContents, optionList);
+}
+
+
+/*
+ * parse_ini_buffer parses the content of a config.ini file.
+ */
+bool
+parse_ini_buffer(const char *filename,
+				 char *fileContents,
+				 IniOption *optionList)
+{
+	IniOption *option;
+
 	/* parse the content of the file as per INI syntax rules */
 	ini_t *ini = ini_load(fileContents, NULL);
 	free(fileContents);

src/bin/pg_autoctl/ini_file.h

@@ -93,6 +93,9 @@ typedef struct IniOption
 	{ INI_END_T, NULL, NULL, NULL, false, false, NULL, -1, -1, NULL, NULL, NULL }
 
 bool read_ini_file(const char *filename, IniOption *opts);
+bool parse_ini_buffer(const char *filename,
+					  char *fileContents,
+					  IniOption *optionList);
 bool ini_validate_options(IniOption *optionList);
 bool ini_set_option_value(IniOption *option, const char *value);
 bool ini_option_to_string(IniOption *option, char *dest, size_t size);

src/bin/pg_autoctl/keeper.c

@@ -805,7 +805,7 @@ keeper_ensure_configuration(Keeper *keeper, bool postgresNotRunningIsOk)
 	 * options being found in our pg_autoctl configuration file or for other
 	 * reasons.
 	 */
-	if (!postgres_add_default_settings(postgres))
+	if (!postgres_add_default_settings(postgres, config->hostname))
 	{
 		log_warn("Failed to edit Postgres configuration after "
 				 "reloading pg_autoctl configuration, "

src/bin/pg_autoctl/keeper_pg_init.c

@@ -905,7 +905,7 @@ create_database_and_extension(Keeper *keeper)
 	 * shared_preload_libraries when dealing with a Citus worker or coordinator
 	 * node.
 	 */
-	if (!postgres_add_default_settings(&initPostgres))
+	if (!postgres_add_default_settings(&initPostgres, config->hostname))
 	{
 		log_error("Failed to add default settings to newly initialized "
 				  "PostgreSQL instance, see above for details");

src/bin/pg_autoctl/monitor_pg_init.c

@@ -343,7 +343,9 @@ monitor_add_postgres_default_settings(Monitor *monitor)
 		}
 	}
 
-	if (!pg_add_auto_failover_default_settings(pgSetup, configFilePath,
+	if (!pg_add_auto_failover_default_settings(pgSetup,
+											   config->hostname,
+											   configFilePath,
 											   monitor_default_settings))
 	{
 		log_error("Failed to add default settings to \"%s\": couldn't "

src/bin/pg_autoctl/pgctl.c

@@ -46,11 +46,13 @@ static bool pg_include_config(const char *configFilePath,
 static bool ensure_default_settings_file_exists(const char *configFilePath,
 												GUC *settings,
 												PostgresSetup *pgSetup,
+												const char *hostname,
 												bool includeTuning);
 static bool prepare_guc_settings_from_pgsetup(const char *configFilePath,
 											  PQExpBuffer config,
 											  GUC *settings,
 											  PostgresSetup *pgSetup,
+											  const char *hostname,
 											  bool includeTuning);
 static void log_program_output(Program prog, int outLogLevel, int errorLogLevel);
 
@@ -731,7 +733,8 @@ find_extension_control_file(const char *pg_ctl, const char *extName)
  */
 bool
 pg_add_auto_failover_default_settings(PostgresSetup *pgSetup,
-									  char *configFilePath,
+									  const char *hostname,
+									  const char *configFilePath,
 									  GUC *settings)
 {
 	bool includeTuning = true;
@@ -750,6 +753,7 @@ pg_add_auto_failover_default_settings(PostgresSetup *pgSetup,
 	if (!ensure_default_settings_file_exists(pgAutoFailoverDefaultsConfigPath,
 											 settings,
 											 pgSetup,
+											 hostname,
 											 includeTuning))
 	{
 		return false;
@@ -870,6 +874,7 @@ static bool
 ensure_default_settings_file_exists(const char *configFilePath,
 									GUC *settings,
 									PostgresSetup *pgSetup,
+									const char *hostname,
 									bool includeTuning)
 {
 	PQExpBuffer defaultConfContents = createPQExpBuffer();
@@ -884,6 +889,7 @@ ensure_default_settings_file_exists(const char *configFilePath,
 										   defaultConfContents,
 										   settings,
 										   pgSetup,
+										   hostname,
 										   includeTuning))
 	{
 		/* errors have already been logged */
@@ -957,6 +963,7 @@ prepare_guc_settings_from_pgsetup(const char *configFilePath,
 								  PQExpBuffer config,
 								  GUC *settings,
 								  PostgresSetup *pgSetup,
+								  const char *hostname,
 								  bool includeTuning)
 {
 	char tuning[BUFSIZE] = { 0 };
@@ -1101,14 +1108,23 @@ prepare_guc_settings_from_pgsetup(const char *configFilePath,
 								  setting->name, pgSetup->citusClusterName);
 			}
 		}
+		else if (streq(setting->name, "citus.local_hostname"))
+		{
+			if (hostname != NULL && !IS_EMPTY_STRING_BUFFER(hostname))
+			{
+				appendPQExpBuffer(config, "%s = '%s'\n",
+								  setting->name, hostname);
+			}
+		}
 		else if (setting->value != NULL &&
 				 !IS_EMPTY_STRING_BUFFER(setting->value))
 		{
 			appendPQExpBuffer(config, "%s = %s\n",
 							  setting->name,
 							  setting->value);
 		}
-		else
+		else if (setting->value == NULL ||
+				 IS_EMPTY_STRING_BUFFER(setting->value))
 		{
 			/*
 			 * Our GUC entry has a NULL (or empty) value. Skip the setting.
@@ -1122,6 +1138,13 @@ prepare_guc_settings_from_pgsetup(const char *configFilePath,
 			 */
 			log_debug("GUC setting \"%s\" has a NULL value", setting->name);
 		}
+		else
+		{
+			/* the GUC setting in the array has not been processed */
+			log_error("BUG: GUC settings \"%s\" has not been processed",
+					  setting->name);
+			return false;
+		}
 	}
 
 	if (includeTuning)
@@ -2004,6 +2027,7 @@ pg_write_recovery_conf(const char *pgdata, ReplicationSource *replicationSource)
 	return ensure_default_settings_file_exists(recoveryConfPath,
 											   recoverySettings,
 											   NULL,
+											   NULL,
 											   includeTuning);
 }
 
@@ -2103,6 +2127,7 @@ pg_write_standby_signal(const char *pgdata,
 	if (!ensure_default_settings_file_exists(standbyConfigFilePath,
 											 recoverySettings,
 											 NULL,
+											 NULL,
 											 includeTuning))
 	{
 		return false;

src/bin/pg_autoctl/pgctl.h

@@ -38,7 +38,8 @@ bool set_pg_ctl_from_config_bindir(PostgresSetup *pgSetup, const char *pg_config
 bool find_pg_config_from_pg_ctl(const char *pg_ctl, char *pg_config, size_t size);
 
 bool pg_add_auto_failover_default_settings(PostgresSetup *pgSetup,
-										   char *configFilePath,
+										   const char *hostname,
+										   const char *configFilePath,
 										   GUC *settings);
 
 bool pg_auto_failover_default_settings_file_exists(PostgresSetup *pgSetup);