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 10:50:36
Message-ID: 4BD6C17C.7090101@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

Fujii Masao wrote:
> On Tue, Apr 27, 2010 at 6:49 PM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> Fujii Masao wrote:
>>>> 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'.
>
> It's OK in pg_start_backup(), but seems NG in pg_stop_backup() since
> it waits until some WAL files have been archived by the archiver. No?

Good point, that logic would need to be changed too. Should it simply
return immediately if archive_mode=off?

>>> + /*
>>> + * 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.
>
> Even if we do only archive recovery, XLogRequestRecoveryConnections
> might be TRUE. Or we need to ensure that the recovery_connection is
> FALSE in the postgresql.conf before starting archive recovery?

Umm, yes, if you have recovery_connnections=on, it means you want hot
standby. And for that you need wal_mode='hot_standby'.

By "it doesn't prevent archive recovery" I meant "you can do traditional
archive recovery without hot standby". It doesn't matter how the WAL is
transported, via the archive or via streaming replication.

--
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 11:24:30 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct
Previous Message Fujii Masao 2010-04-27 10:12:33 Re: Re: [COMMITTERS] pgsql: Make CheckRequiredParameterValues() depend upon correct

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2010-04-27 11:06:10 Re: CP949 for EUC-KR?
Previous Message Takahiro Itagaki 2010-04-27 10:27:40 CP949 for EUC-KR?