Re: Patch for reserved connections for replication users - Mailing list pgsql-hackers
From | Gibheer |
---|---|
Subject | Re: Patch for reserved connections for replication users |
Date | |
Msg-id | 20131010183657.3fe5edf8@linse.fritz.box Whole thread Raw |
In response to | Re: Patch for reserved connections for replication users (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
Re: Patch for reserved connections for replication users
|
List | pgsql-hackers |
On Thu, 10 Oct 2013 09:55:24 +0530 Amit Kapila <amit.kapila16@gmail.com> wrote: > On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org> > wrote: > > On Mon, 7 Oct 2013 11:39:55 +0530 > > Amit Kapila <amit.kapila16@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! > > Is there any specific reason why you moved this patch to next > CommitFest? > > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com > Mike asked me about the status of the patch a couple days back and I didn't think I would be able to work on the patch so soon again. That's why I told him to just move the patch into the next commitfest.
pgsql-hackers by date: