On Tue, Mar 1, 2016 at 10:02 AM, Peter Eisentraut <peter_e@gmx.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
