On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapil...@gmail.com> wrote:
> Robert Haas wrote:
> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> <andres(at)2ndquadrant(dot)com> wrote:
> >>> Hmm. It seems like this match is making MaxConnections no longer
> >>> mean the maximum number of connections, but rather the maximum
> >>> number of non-replication connections. I don't think I support
> >>> that definitional change, and I'm kinda surprised if this is
> >>> sufficient to implement it anyway (e.g. see InitProcGlobal()).
> >
> >> I don't think the implementation is correct, but why don't you
> >> like the definitional change? The set of things you can do from
> >> replication connections are completely different from a normal
> >> connection. So using separate "pools" for them seems to make sense.
> >> That they end up allocating similar internal data seems to be an
> >> implementation detail to me.
>
> > Because replication connections are still "connections". If I tell
> > the system I want to allow 100 connections to the server, it should
> > allow 100 connections, not 110 or 95 or any other number.
>
> I think that to reserve connections for replication, mechanism similar
> to superuser_reserved_connections be used rather than auto vacuum
> workers or background workers.
> This won't change the definition of MaxConnections. Another thing is
> that rather than introducing new parameter for replication reserved
> connections, it is better to use max_wal_senders as it can serve the
> purpose.
>
> Review for replication_reserved_connections-v2.patch, considering we
> are going to use mechanism similar to superuser_reserved_connections
> and won't allow definition of MaxConnections to change.
>
> 1. /* the extra unit accounts for the autovacuum launcher */
> MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
> - + max_worker_processes;
> + + max_worker_processes + max_wal_senders;
>
> Above changes are not required.
>
> 2.
> + if ((!am_superuser && !am_walsender) &&
> ReservedBackends > 0 &&
> !HaveNFreeProcs(ReservedBackends))
>
> Change the check as you have in patch-1 for
> ReserveReplicationConnections
>
> if (!am_superuser &&
> (max_wal_senders > 0 || ReservedBackends > 0) &&
> !HaveNFreeProcs(max_wal_senders + ReservedBackends))
> ereport(FATAL,
> (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
> errmsg("remaining connection slots are reserved for replication and
> superuser connections")));
>
> 3. In guc.c, change description of max_wal_senders similar to
> superuser_reserved_connections at below place:
> {"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
> gettext_noop("Sets the maximum number of simultaneously running WAL
> sender processes."),
>
> 4. With this approach, there is no need to change InitProcGlobal(), as
> it is used to keep track bgworkerFreeProcs and autovacFreeProcs, for
> others it use freeProcs.
>
> 5. Below description in config.sgml needs to be changed for
> superuser_reserved_connections to include the effect of max_wal_enders
> in reserved connections.
> Whenever the number of active concurrent connections is at least
> max_connections minus superuser_reserved_connections, new connections
> will be accepted only for superusers, and no new replication
> connections will be accepted.
>
> 6. Also similar description should be added to max_wal_senders
> configuration parameter.
>
> 7. Below comment needs to be updated, as it assumes only superuser
> reserved connections as part of MaxConnections limit.
> /*
> * ReservedBackends is the number of backends reserved for superuser
> use.
> * This number is taken out of the pool size given by MaxBackends so
> * number of backend slots available to non-superusers is
> * (MaxBackends - ReservedBackends). Note what this really means is
> * "if there are <= ReservedBackends connections available, only
> superusers
> * can make new connections" --- pre-existing superuser connections
> don't
> * count against the limit.
> */
> int ReservedBackends;
>
> 8. Also we can add comment on top of definition for max_wal_senders
> similar to ReservedBackends.
>
>
> With Regards,
> Amit Kapila.
> EnterpriseDB: http://www.enterprisedb.com
>
Hi,
I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!
So this patch uses max_wal_senders together with the idea of the first
patch I sent. The error messages are also adjusted to make it obvious,
how it is supposed to be and all checks work, as far as I could tell.
regards,
Stefan Radomski
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e8e8e6f..a67cd2f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -519,7 +519,7 @@ include 'filename'
<varname>max_connections</> minus
<varname>superuser_reserved_connections</varname>, new
connections will be accepted only for superusers, and no
- new replication connections will be accepted.
+ new WAL or other connections will be accepted.
</para>
<para>
@@ -2167,7 +2167,13 @@ include 'filename'
Specifies the maximum number of concurrent connections from
standby servers or streaming base backup clients (i.e., the
maximum number of simultaneously running WAL sender
- processes). The default is zero, meaning replication is
+ processes). The number of WAL senders is also reserved from
+ <xref linkend="guc-max-connections">. Whenever the number of connections
+ is at least <varname>max_connections</varname> minus
+ <varname>superuser_reserved_connections</varname>
+ <varname>max_wal_senders</varname> only superuser and WAL connections
+ are allowed.
+ The default is zero, meaning replication is
disabled. WAL sender processes count towards the total number
of connections, so the parameter cannot be set higher than
<xref linkend="guc-max-connections">. This parameter can only
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index c9a8a8f..c341d2e 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -198,10 +198,10 @@ char *ListenAddresses;
* ReservedBackends is the number of backends reserved for superuser use.
* This number is taken out of the pool size given by MaxBackends so
* number of backend slots available to non-superusers is
- * (MaxBackends - ReservedBackends). Note what this really means is
- * "if there are <= ReservedBackends connections available, only superusers
- * can make new connections" --- pre-existing superuser connections don't
- * count against the limit.
+ * (MaxBackends - ReservedBackends - max_wal_senders). Note what this really
+ * means is "if there are <= ReservedBackends connections available, only
+ * superusers can make new connections" --- pre-existing superuser connections
+ * don't count against the limit.
*/
int ReservedBackends;
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index afd559d..fd023d7 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -94,7 +94,13 @@ bool am_cascading_walsender = false; /* Am I cascading WAL to
* another standby ? */
/* User-settable parameters for walsender */
-int max_wal_senders = 0; /* the maximum number of concurrent walsenders */
+/*
+ * This is the maximum number of concurrent wal senders and the amount of
+ * reserved slots from max_connections for wal senders. If the number of free
+ * slots (max_connections - max_wal_senders) is depleted, only superusers and
+ * wal senders can connect.
+ */
+int max_wal_senders = 0;
int wal_sender_timeout = 60 * 1000; /* maximum time to send one
* WAL data message */
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 2c7f0f1..2ebe641 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -700,17 +700,26 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username,
}
/*
- * The last few connections slots are reserved for superusers. Although
- * replication connections currently require superuser privileges, we
- * don't allow them to consume the reserved slots, which are intended for
- * interactive use.
+ * The last few connections slots are reserved for superusers and wal_senders.
+ */
+ if (!am_superuser &&
+ (max_wal_senders > 0 || ReservedBackends > 0) &&
+ !HaveNFreeProcs(max_wal_senders + ReservedBackends))
+ ereport(FATAL,
+ (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
+ errmsg("remaining connection slots are reserved for replication and superuser connections")));
+
+ /*
+ * Although replication connections currently require superuser privileges, we
+ * don't allow them to consume the superuser reserved slots, which are
+ * intended for interactive use.
*/
if ((!am_superuser || am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
- errmsg("remaining connection slots are reserved for non-replication superuser connections")));
+ errmsg("remaining connection slots are reserved for superuser connections")));
/*
* If walsender, we don't want to connect to any particular database. Just
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ddbeb34..836b369 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -2063,7 +2063,7 @@ static struct config_int ConfigureNamesInt[] =
{
/* see max_connections */
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
- gettext_noop("Sets the maximum number of simultaneously running WAL sender processes."),
+ gettext_noop("Sets the maximum number wal sender connections and reserves them."),
NULL
},
&max_wal_senders,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
