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

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Date: 2010-04-27 09:49:06
Message-ID: 4BD6B312.3040101@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers 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

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Fujii Masao 2010-04-27 10:12:33 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Previous Message Fujii Masao 2010-04-27 09:34:10 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2010-04-27 10:12:33 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Previous Message Fujii Masao 2010-04-27 09:34:10 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct