From: Heikki Linnakangas on 27 Apr 2010 13:18 Tom Lane wrote: > Heikki Linnakangas <heikki.linnakangas(a)enterprisedb.com> writes: >> Ok, here's a patch that includes the changes to add new wal_mode GUC >> (http://archives.postgresql.org/message-id/4BD581A6.60602(a)enterprisedb.com), > > I haven't read this in any detail, but why does it add inclusion of > pg_control.h to xlog.h? I don't see any reason for that in the actual > changes in xlog.h. I put the enum for wal_mode to pg_control.h, so that it's available to pg_controlinfo.c without #including xlog.h there. The XLogArchivingActive() macro in xlog.h needs the enum values: #define XLogArchivingActive() (XLogArchiveMode && wal_mode >= WAL_MODE_ARCHIVE I'm all ears for better suggestions, I didn't like that much either. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 27 Apr 2010 15:00 Tom Lane wrote: > How about putting the enum {} declaration in xlog.h, and making the > field in pg_control.h just be declared "int"? I tried that at first, but the problem was with pg_controldata.c. In bin/. I wanted it to print wal_mode in human-readable format, so it needed the values of the enum from somewhere. I tried to "#include <access/xlog.h>" in pg_controlinfo.c, but got a bunch of errors. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 28 Apr 2010 03:43 Tom Lane wrote: > Alvaro Herrera <alvherre(a)commandprompt.com> writes: >> Hmm, AFAICS the problem with controldata is that it uses postgres_fe.h >> instead of postgres.h. It's a bit of a stretch to use the latter, but >> maybe that's a better solution? After all, it *is* poking into the >> backend internals. > > I seem to recall that Solaris had problems with that due to dtrace > support or something? However, we are doing it in pg_resetxlog, > so I suppose it's ok for pg_controldata as well. Ok, did that. Here's an updated patch: * made incorrect combinations of wal_mode and archive_mode/max_wal_senders throw an ERROR instead of WARNING * renamed wal_mode to wal_level * reworded the documentation changes a bit This doesn't contain any changes to pg_start_backup() yet, that's a separate issue and still under discussion. At commit, should I bump catversion, or PG_CONTROL_VERSION, or both? The patch replaces the unlogged-operation WAL record with a record containing current parameter values, and it changes pg_control. I'm guessing both. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
From: Heikki Linnakangas on 28 Apr 2010 12:14 Ok, I've finally committed the patch, using wal_level as the name of the GUC. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Heikki Linnakangas on 29 Apr 2010 05:55 Simon Riggs wrote: > On Thu, 2010-04-29 at 10:55 +0300, Heikki Linnakangas wrote: > >> Should we change the default to recovery_connections=off ? It is a quite >> big change in default behavior over previous releases that hot standby >> is enabled by default, so maybe that would be the right thing to do anyway. >> >> Or maybe add a hint to the above, "disable hot standby by setting >> recovery_connections=off, or set wal_level='hot_standby' in the primary" > > I've been waiting for this suggestion, a clear knock-on effect from your > earlier changes. If we just have a static default its bad either way. > > The most sensible way is to make the default act intelligently depending > upon what it finds in the control file. If WAL contains hot_standby > info, then recovery_connections should be on by default, though can be > turned off explicitly if required. If WAL does not contain hot_standby > info we then we throw a WARNING and continue as if recovery_connections > = off, or if recovery_connections is specified explicitly then we should > throw an ERROR so that the user knows it won't be possible to use this. > I think that means we'd need to change recovery_connections to have 2 or > 3 values, but non-boolean: > recovery_connections = auto (default) | off > or > recovery_connections = auto (default) | on | off > Seems overly complicated. It would be simpler to just set it 'off' by default. If it's 'off' by default, and you want to use hot standby, you will notice quickly that the server doesn't accept connections, and you switch it on. If it's 'on' (or 'auto'), you might not realize that your standby server is accepting connections, when you only wanted to set it up as a warm standby server for high availability. The 'auto' mode just makes it more surprising. Connections might be allowed at first, but when someone switches wal_level in the primary for some reason, your reporting standby is up, but no longer allows connections. And vice versa, if you set up a warm standby server for high availability where you don't want to allow connections, and someone flips the wal_level switch in the master, the standby suddenly starts accepting connections. I can't imagine a situation where "allow connections if the WAL allows it" would be a sensible setting. If there is one, we could have an 'auto' mode, but not as the default. > and I would suggest removing it from postgresql.conf.sample -1. Why try to hide it? -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
|
Next
|
Last
Pages: 1 2 3 Prev: Differential backup Next: pgsql: MakeCheckRequiredParameterValues() depend upon correct |