Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct

Fujii Masao wrote:
> config.sgml
>> <literal>on</literal>.  It is thought that there is little
>> measurable difference in performance from using this feature, so
>> feedback is welcome if any production impacts are noticeable.
>> It is likely that this parameter will be removed in later releases.
> 
> Is this description still required for recovery_connections?

Hmm, I guess it was referring to setting recovery_connections in the
master, I don't see us removing that option from the standby in the
future. recovery_connections in the master is being replaced with the
wal_mode setting, so I guess that's not required anymore.

>> if (!XLogArchivingActive())
>>   ereport(ERROR,
>>     (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>>      errmsg("WAL archiving is not active"),
>>      errhint("archive_mode must be enabled at server start.")));
> 
> You need to change the error messages which refer to archive_mode,
> like the above.

Hmm, I think we should change not only the error message, but the logic
too. There's two related checks there:

> 	if (!XLogArchivingActive())
> 		ereport(ERROR,
> 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> 				 errmsg("WAL archiving is not active"),
> 				 errhint("archive_mode must be enabled at server start.")));
> 
> 	if (!XLogArchiveCommandSet())
> 		ereport(ERROR,
> 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> 				 errmsg("WAL archiving is not active"),
> 				 errhint("archive_command must be defined before "
> 						 "online backups can be made safely.")));

You can use streaming replication too to transport the WAL generated
during the backup, so I think we should just check that wal_mode>='archive'.

> + /*
> +  * For Hot Standby, the WAL must be generated with 'hot_standby' mode,
> +  * and we must have at least as many backend slots as the primary.
> +  */
> + if (InArchiveRecovery && XLogRequestRecoveryConnections)
> + {
> +   if (ControlFile->wal_mode < WAL_MODE_HOT_STANDBY)
> +       ereport(ERROR,
> +              (errmsg("recovery connections cannot start because
> wal_mode was not set to 'hot_standby' on the WAL source server")));
> 
> This seems to always prevent the server from doing an archive recovery
> since wal_mode is expected to be WAL_MODE_ARCHIVE in that case.

No, it doesn't prevent archive recovery. It only prevents hot standby if
wal_mode was not 'hot_standby' in the master. I think you missed the "&&
XLogRequestRecoveryConnections" condition above.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com