Re: pgsql: Integrate recovery.conf into postgresql.conf

Greetings,

* Sergei Kornilov (sk@zsrv.org) wrote:
> Updated patch attached:
> - added testcase to verify database does not start with multiple recovery targets
> - recovery_target = immediate was replaced with recovery_target_immediate bool GUC

I'd encourage you to look through the diff after you're finished hacking
before sending it to the list, in case things get left in that should be
removed, as below...

> diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
> index c80b14e..cd29606 100644
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -261,13 +261,21 @@ static bool restoredFromArchive = false;
>  static char *replay_image_masked = NULL;
>  static char *master_image_masked = NULL;
>  
> +/* recovery_target* original GUC values */
> +char *recovery_target_string;

This shouldn't be here now, should it?

> +char *recovery_target_xid_string;
> +char *recovery_target_time_string;
> +char *recovery_target_name_string;
> +char *recovery_target_lsn_string;

>  /* options formerly taken from recovery.conf for archive recovery */

Shouldn't all of the above be listed under this comment..?

>  char	   *recoveryRestoreCommand = NULL;
>  char	   *recoveryEndCommand = NULL;
>  char	   *archiveCleanupCommand = NULL;
> -RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
> +static RecoveryTargetType recoveryTarget = RECOVERY_TARGET_UNSET;
>  bool		recoveryTargetInclusive = true;
>  int			recoveryTargetAction = RECOVERY_TARGET_ACTION_PAUSE;
> +bool 		recoveryTargetImmediate;

Seems like this should, at least, be close to the char*'s that are
defining the other ways to specify a recovery targer.

>  TransactionId recoveryTargetXid;
>  TimestampTz recoveryTargetTime;
>  char	   *recoveryTargetName;
> @@ -5381,9 +5389,42 @@ readRecoverySignalFile(void)
>  static void
>  validateRecoveryParameters(void)
>  {
> +	uint8 targetSettingsCount = 0;
> +
>  	if (!ArchiveRecoveryRequested)
>  		return;
>  
> +	/* Check for mutually exclusive target actions */
> +	if (recoveryTargetImmediate)
> +	{
> +		++targetSettingsCount;
> +		recoveryTarget = RECOVERY_TARGET_IMMEDIATE;
> +	}
> +	if (strcmp(recovery_target_name_string, "") != 0)
> +	{
> +		++targetSettingsCount;
> +		recoveryTarget = RECOVERY_TARGET_NAME;
> +	}
> +	if (strcmp(recovery_target_lsn_string, "") != 0)
> +	{
> +		++targetSettingsCount;
> +		recoveryTarget = RECOVERY_TARGET_LSN;
> +	}
> +	if (strcmp(recovery_target_xid_string, "") != 0)
> +	{
> +		++targetSettingsCount;
> +		recoveryTarget = RECOVERY_TARGET_XID;
> +	}
> +	if (strcmp(recovery_target_time_string, "") != 0)
> +	{
> +		++targetSettingsCount;
> +		recoveryTarget = RECOVERY_TARGET_TIME;
> +	}
> +	if (targetSettingsCount > 1)
> +		ereport(FATAL,
> +				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> +				 errmsg("can not specify multiple recovery targets")));

Doesn't look like you changed this based on my prior comment..?

Thanks!

Stephen