Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Date
Msg-id 4BD6B312.3040101@enterprisedb.com
Whole thread Raw
In response to Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct  (Fujii Masao <masao.fujii@gmail.com>)
List pgsql-hackers
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


pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Next
From: Fujii Masao
Date:
Subject: Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct