On Tue, Mar 1, 2016 at 10:02 AM, Peter Eisentraut <pete...@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_mgc3ahfzrioneg4e5ccegvzc0b6...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers