Re: remove wal_level archive

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Peter Eisentraut <peter_e(at)gmx(dot)net>
Cc: Simon Riggs <simon(at)2ndquadrant(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: remove wal_level archive
Date: 2016-03-01 06:11:24
Message-ID: CAB7nPqRbE-H8aSWzFyoBz-P3-+N_y0hmo+upzYnskuKyaKtybg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 1, 2016 at 10:02 AM, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> On 2/8/16 7:34 AM, Michael Paquier wrote:
>> - if (ControlFile->wal_level < WAL_LEVEL_HOT_STANDBY)
>> + if (ControlFile->wal_level < WAL_LEVEL_REPLICA)
>> Upthread it was mentioned that switching to an approach where enum
>> values are directly listed would be better. The target of an extra
>> patch on top of this one?
>
> I'm not sure what is meant by that.

The following for example, aka using only equal comparators with the
name of wal_level used:
if (ArchiveRecoveryRequested && EnableHotStandby)
{
- if (ControlFile->wal_level < WAL_LEVEL_HOT_STANDBY)
+ if (ControlFile->wal_level == WAL_LEVEL_ARCHIVE ||
+ ControlFile->wal_level == WAL_LEVEL_MINIMAL)
ereport(ERROR,
But that's nitpicking (this was mentioned upthread), and not something
this patch should care about.

>> - if (wal_level < WAL_LEVEL_ARCHIVE)
>> - ereport(ERROR,
>> -
>> (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
>> - errmsg("replication slots can only be
>> used if wal_level >= archive")));
>> We should still forbid the creation of replication slots if wal_level = minimal.
>
> I think I took this out because you actually can't get to that check,
> but I put it back in because it seems better for clarity.

It is possible to hit it, take for example the following set of
parameters in postgresql.conf:
max_replication_slots = 3
wal_level = minimal
max_wal_senders = 0
=# select pg_create_physical_replication_slot('toto');
ERROR: 55000: replication slots can only be used if wal_level >= archive
LOCATION: CheckSlotRequirements, slot.c:766

If this patch gets committed before the one improving the checkpoint
skip logic when wal_level >= hot_standby regarding standby snapshots
(http://www.postgresql.org/message-id/CAB7nPqQAaB0v25tt4SJ_mGc3aHfZrionEG4E5cceGVZc0B6QyA@mail.gmail.com)
something to not forget is that there will be a regression: on idle
systems checkpoints won't be skipped anymore, which is now what
happens with wal_level = archive on HEAD.

Except this last concern, the patch is in a nice shape, and does what
it says it does.
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2016-03-01 06:24:12 Re: [REVIEW] In-core regression tests for replication, cascading, archiving, PITR, etc.
Previous Message Michael Paquier 2016-03-01 05:38:23 OOM in libpq and infinite loop with getCopyStart()