Re: [HACKERS] Patch for reserved connections for replication users
On 15-10-2013 14:34, Josh Berkus wrote: > On 10/15/2013 07:36 AM, Robert Haas wrote: >> On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund >> wrote: >>> Josh said we should treat replication connections in a separate "pool" >>> from normal database connections, right? So you withdraw your earlier >>> objection to that? >> >> I don't think that's what he said. Here's what I was referring to: > > To clarify: I do, indeed, support the idea of treating replication > connections as a pool outside of max_connections. Here's why: > +1. I've already faced the same problem Josh described. Even if it is documented (as in max_wal_senders), it is not easy to figure out why you can't connect (check 2 parameters and query 2 views). Also, the 'check connections' task is so complicated in a monitoring tool. Let's separate the replication connections in another pool (that is not related to user connections). We already have the infrastructure to limit replication connections (max_wal_senders), let's use it. > FATAL: connection limit exceeded for non-superusers > > SHOW max_connections; > > 100 > > SELECT COUNT(*) FROM pg_stat_activity; > > 94 > > SHOW superuser_reserved_connections; > > 3 > > > > ... search around quite a bit, eventually figure out that you have > three replication connections open. We've already set up an illogical > and hard-to-troubleshoot situation where replication connections do not > appear in pg_stat_activity, yet they are counted against max_connections. > Another situation is when you can't run pg_basebackup because automated routines got all of the available connections and the replication user is not a superuser (even if replication user is a superuser, if some app run as superuser there won't be slots available). -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Sun, Oct 20, 2013 at 1:26 AM, Gibheer wrote: > On Sat, 19 Oct 2013 12:09:57 +0530 > Amit Kapila wrote: > >> On Thu, Oct 17, 2013 at 8:57 AM, Amit Kapila >> wrote: >> > On Wed, Oct 16, 2013 at 4:30 AM, Gibheer >> > wrote: >> >> On Mon, 14 Oct 2013 11:52:57 +0530 >> >> Amit Kapila wrote: >> >> >> >>> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer >> >>> wrote: >> >>> > On Sun, 13 Oct 2013 11:38:17 +0530 >> >>> > Amit Kapila wrote: >> >>> > >> >>> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer >> >>> >> wrote: >> >>> >> > On Mon, 7 Oct 2013 11:39:55 +0530 >> >>> >> > Amit Kapila wrote: >> >>> >> >> Robert Haas wrote: >> >>> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund >> >>> >> >> wrote: >> >> I would be glad, if you could also test the patch again, as I'm >> >> nearly code blind after testing it for 4 hours. >> >> I had the problem, that I could not attach as many replication >> >> connections as I wanted, as they were denied as normal >> >> connections. I think I got it fixed, but I'm not 100% sure at the >> >> moment. After some sleep, I will read the code again and test it >> >> again, to make sure, it really does what it is supposed to do. >> > >> > You have forgotten to attach the patch. However, now it is important >> > to first get the consensus on approach to do this feature, currently >> > there are 3 approaches: >> > 1. Have replication_reserved_connections as a separate parameter to >> > reserve connections for replication >> > 2. Consider max_wal_sender to reserve connections for replication >> > 3. Treat replication connections as a pool outside max_connections >> > >> > Apart from above approaches, we need to think how user can view the >> > usage of connections, as pg_stat_activity doesn't show replication >> > connections, so its difficult for user to see how the connections >> > are used. >> > >> > I am really not sure what is best way to goahead from here, but I >> > think it might be helpful if we can study some use cases or how >> > other databases solve this problem. >> >> Today I spent some time seeing how other databases (in particular >> MySQL) achieve it. There seems to be no separate way to configure >> replication connections, rather if user faced with >> too_many_connections >> (https://dev.mysql.com/doc/refman/5.5/en/too-many-connections.html), >> they allow one spare connection (super user connection) to check what >> all connections are doing, it seems all connections can be viewed >> through one common command Show ProcessList >> (https://dev.mysql.com/doc/refman/5.5/en/show-processlist.html). >> >> By above, I don't mean that we should only do what other databases >> have done, rather it is towards trying to find a way which can be >> useful for users of Postgresql. >> >> Your views/thoughts? >> >> With Regards, >> Amit Kapila. >> EnterpriseDB: http://www.enterprisedb.com >> > > Hi, > > I have accessto MySQL and PostgreSQL at work and it is true, that MySQL > has not separate pools. It also happend to us, that we lost connection > from a slave and it was unable to get back into replication on MySQL > and Postgres, because of some stupid applications. > One difference is, like you said, that replication connections are > listed in `show processlist`, where replication connections in postgres > are listed in a seperate view from the rest of the connections. I think > the postgres way is the better in this case, as the picture of the > replication state of the complete cluster can be viewed by one select > on the master. In MySQL it needs one SQL on each slave. Going either way (separate management of replication connections or unified max_connections), user has to understand how to configure the system, so that it serves his purpose. Here I think the important thing is to decide which way it would be easy for users to understand and configure the system. As an user, I would be happy with one parameter (max_connections) rather than having multiple parameters for connection management and understand each one separately to configure the system. However here many users would be more comfortable if there are multiple parameters for configuring the system. I was not sure which way users would like to configure connection management and neither we had consensus to proceed, thats why I had checked other database to know how users are configuring connection management in database and it seems to me that many users are using single parameter. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Sat, 19 Oct 2013 12:09:57 +0530 Amit Kapila wrote: > On Thu, Oct 17, 2013 at 8:57 AM, Amit Kapila > wrote: > > On Wed, Oct 16, 2013 at 4:30 AM, Gibheer > > wrote: > >> On Mon, 14 Oct 2013 11:52:57 +0530 > >> Amit Kapila wrote: > >> > >>> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer > >>> wrote: > >>> > On Sun, 13 Oct 2013 11:38:17 +0530 > >>> > Amit Kapila wrote: > >>> > > >>> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer > >>> >> wrote: > >>> >> > On Mon, 7 Oct 2013 11:39:55 +0530 > >>> >> > Amit Kapila wrote: > >>> >> >> Robert Haas wrote: > >>> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund > >>> >> >> wrote: > >> I would be glad, if you could also test the patch again, as I'm > >> nearly code blind after testing it for 4 hours. > >> I had the problem, that I could not attach as many replication > >> connections as I wanted, as they were denied as normal > >> connections. I think I got it fixed, but I'm not 100% sure at the > >> moment. After some sleep, I will read the code again and test it > >> again, to make sure, it really does what it is supposed to do. > > > > You have forgotten to attach the patch. However, now it is important > > to first get the consensus on approach to do this feature, currently > > there are 3 approaches: > > 1. Have replication_reserved_connections as a separate parameter to > > reserve connections for replication > > 2. Consider max_wal_sender to reserve connections for replication > > 3. Treat replication connections as a pool outside max_connections > > > > Apart from above approaches, we need to think how user can view the > > usage of connections, as pg_stat_activity doesn't show replication > > connections, so its difficult for user to see how the connections > > are used. > > > > I am really not sure what is best way to goahead from here, but I > > think it might be helpful if we can study some use cases or how > > other databases solve this problem. > > Today I spent some time seeing how other databases (in particular > MySQL) achieve it. There seems to be no separate way to configure > replication connections, rather if user faced with > too_many_connections > (https://dev.mysql.com/doc/refman/5.5/en/too-many-connections.html), > they allow one spare connection (super user connection) to check what > all connections are doing, it seems all connections can be viewed > through one common command Show ProcessList > (https://dev.mysql.com/doc/refman/5.5/en/show-processlist.html). > > By above, I don't mean that we should only do what other databases > have done, rather it is towards trying to find a way which can be > useful for users of Postgresql. > > Your views/thoughts? > > With Regards, > Amit Kapila. > EnterpriseDB: http://www.enterprisedb.com > Hi, I have accessto MySQL and PostgreSQL at work and it is true, that MySQL has not separate pools. It also happend to us, that we lost connection from a slave and it was unable to get back into replication on MySQL and Postgres, because of some stupid applications. One difference is, like you said, that replication connections are listed in `show processlist`, where replication connections in postgres are listed in a seperate view from the rest of the connections. I think the postgres way is the better in this case, as the picture of the replication state of the complete cluster can be viewed by one select on the master. In MySQL it needs one SQL on each slave. On the other hand, wor on logical replication is done, which will have predefined slots for it (http://wiki.postgresql.org/wiki/BDR_User_Guide#max_logical_slots). This will also consume slots from max_wal_senders (http://wiki.postgresql.org/wiki/BDR_User_Guide#max_wal_senders). With that, I think the best approach is to build a pool around replication only connections, despite it's possible kind. Information about them will only be available through pg_stat_replication. When a connection limit is hit, it is clear wether it is a normal connection or a replication connection and where the user should look for further information about it. Also nobody would have to calculate how many connections would have to be reserved for what service. I have yet to take a look how background worker connections are handled (seperate pool or unified with normal connections), because for them the same limitations apply. We want them running despite the load from applications or users. I will report back, when I had more time to look into it. regards, Stefan Radomski -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Thu, Oct 17, 2013 at 8:57 AM, Amit Kapila wrote: > On Wed, Oct 16, 2013 at 4:30 AM, Gibheer wrote: >> On Mon, 14 Oct 2013 11:52:57 +0530 >> Amit Kapila wrote: >> >>> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer >>> wrote: >>> > On Sun, 13 Oct 2013 11:38:17 +0530 >>> > Amit Kapila wrote: >>> > >>> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer >>> >> wrote: >>> >> > On Mon, 7 Oct 2013 11:39:55 +0530 >>> >> > Amit Kapila wrote: >>> >> >> Robert Haas wrote: >>> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund >>> >> >> wrote: >> I would be glad, if you could also test the patch again, as I'm nearly >> code blind after testing it for 4 hours. >> I had the problem, that I could not attach as many replication >> connections as I wanted, as they were denied as normal connections. I >> think I got it fixed, but I'm not 100% sure at the moment. >> After some sleep, I will read the code again and test it again, to make >> sure, it really does what it is supposed to do. > > You have forgotten to attach the patch. However, now it is important > to first get the consensus on approach to do this feature, currently > there are 3 approaches: > 1. Have replication_reserved_connections as a separate parameter to > reserve connections for replication > 2. Consider max_wal_sender to reserve connections for replication > 3. Treat replication connections as a pool outside max_connections > > Apart from above approaches, we need to think how user can view the > usage of connections, as pg_stat_activity doesn't show replication > connections, so its difficult for user to see how the connections are > used. > > I am really not sure what is best way to goahead from here, but I > think it might be helpful if we can study some use cases or how other > databases solve this problem. Today I spent some time seeing how other databases (in particular MySQL) achieve it. There seems to be no separate way to configure replication connections, rather if user faced with too_many_connections (https://dev.mysql.com/doc/refman/5.5/en/too-many-connections.html), they allow one spare connection (super user connection) to check what all connections are doing, it seems all connections can be viewed through one common command Show ProcessList (https://dev.mysql.com/doc/refman/5.5/en/show-processlist.html). By above, I don't mean that we should only do what other databases have done, rather it is towards trying to find a way which can be useful for users of Postgresql. Your views/thoughts? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Wed, Oct 16, 2013 at 4:30 AM, Gibheer wrote: > On Mon, 14 Oct 2013 11:52:57 +0530 > Amit Kapila wrote: > >> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer >> wrote: >> > On Sun, 13 Oct 2013 11:38:17 +0530 >> > Amit Kapila wrote: >> > >> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer >> >> wrote: >> >> > On Mon, 7 Oct 2013 11:39:55 +0530 >> >> > Amit Kapila wrote: >> >> >> Robert Haas wrote: >> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund >> >> >> 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. >> >> >> >> >> >> >> > >> >> > 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. >> >> >> >> If I understand correctly, now the patch has implementation such >> >> that a. if the number of connections left are (ReservedBackends + >> >> max_wal_senders), then only superusers or replication connection's >> >> will be allowed >> >> b. if the number of connections left are ReservedBackend, then only >> >> superuser connections will be allowed. >> > >> > That is correct. >> > >> >> So it will ensure that max_wal_senders is used for reserving >> >> connection slots from being used by non-super user connections. I >> >> find new usage of max_wal_senders acceptable, if anyone else thinks >> >> otherwise, please let us know. >> >> >> >> >> >> 1. >> >> +superuser_reserved_connections >> >> +max_wal_senders only superuser and WAL >> >> connections >> >> +are allowed. >> >> >> >> Here minus seems to be missing before max_wal_senders and I think >> >> it will be better to use replication connections rather than WAL >> >> connections. >> > >> > This is fixed. >> > >> >> 2. >> >> -new replication connections will be accepted. >> >> +new WAL or other connections will be accepted. >> >> >> >> I think as per new implementation, we don't need to change this >> >> line. >> > >> > I reverted that change. >> > >> >> 3. >> >> + * reserved slots from max_connections for wal senders. If the >> >> number of free >> >> + * slots (max_connections - max_wal_senders) is depleted. >> >> >> >> Above calculation (max_connections - max_wal_senders) needs to >> >> include super user reserved connections. >> > >> > My first thought was, that I would not add it here. When superuser >> > reserved connections are not set, then only max_wal_senders would >> > count. >> > But you are right, it has to be set, as 3 connections are reserved >> > by default for superusers. >> >> + * slots (max_connections - superuser_reserved_connections - >> max_wal_senders) here it should be ReservedBackends rather than >> superuser_reserved_connections. > > fixed > >> >> 4. >> >> + /* >> >> + * 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(Reser
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, 14 Oct 2013 11:52:57 +0530
Amit Kapila wrote:
> On Sun, Oct 13, 2013 at 2:08 PM, Gibheer
> wrote:
> > On Sun, 13 Oct 2013 11:38:17 +0530
> > Amit Kapila wrote:
> >
> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
> >> wrote:
> >> > On Mon, 7 Oct 2013 11:39:55 +0530
> >> > Amit Kapila wrote:
> >> >> Robert Haas wrote:
> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> >> >> 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.
> >> >>
> >>
> >> >
> >> > 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.
> >>
> >> If I understand correctly, now the patch has implementation such
> >> that a. if the number of connections left are (ReservedBackends +
> >> max_wal_senders), then only superusers or replication connection's
> >> will be allowed
> >> b. if the number of connections left are ReservedBackend, then only
> >> superuser connections will be allowed.
> >
> > That is correct.
> >
> >> So it will ensure that max_wal_senders is used for reserving
> >> connection slots from being used by non-super user connections. I
> >> find new usage of max_wal_senders acceptable, if anyone else thinks
> >> otherwise, please let us know.
> >>
> >>
> >> 1.
> >> +superuser_reserved_connections
> >> +max_wal_senders only superuser and WAL
> >> connections
> >> +are allowed.
> >>
> >> Here minus seems to be missing before max_wal_senders and I think
> >> it will be better to use replication connections rather than WAL
> >> connections.
> >
> > This is fixed.
> >
> >> 2.
> >> -new replication connections will be accepted.
> >> +new WAL or other connections will be accepted.
> >>
> >> I think as per new implementation, we don't need to change this
> >> line.
> >
> > I reverted that change.
> >
> >> 3.
> >> + * reserved slots from max_connections for wal senders. If the
> >> number of free
> >> + * slots (max_connections - max_wal_senders) is depleted.
> >>
> >> Above calculation (max_connections - max_wal_senders) needs to
> >> include super user reserved connections.
> >
> > My first thought was, that I would not add it here. When superuser
> > reserved connections are not set, then only max_wal_senders would
> > count.
> > But you are right, it has to be set, as 3 connections are reserved
> > by default for superusers.
>
> + * slots (max_connections - superuser_reserved_connections -
> max_wal_senders) here it should be ReservedBackends rather than
> superuser_reserved_connections.
fixed
> >> 4.
> >> + /*
> >> + * 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 conn
Re: [HACKERS] Patch for reserved connections for replication users
On 10/15/2013 07:36 AM, Robert Haas wrote: > On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund > wrote: >> Josh said we should treat replication connections in a separate "pool" >> from normal database connections, right? So you withdraw your earlier >> objection to that? > > I don't think that's what he said. Here's what I was referring to: To clarify: I do, indeed, support the idea of treating replication connections as a pool outside of max_connections. Here's why: FATAL: connection limit exceeded for non-superusers SHOW max_connections; 100 SELECT COUNT(*) FROM pg_stat_activity; 94 SHOW superuser_reserved_connections; 3 ... search around quite a bit, eventually figure out that you have three replication connections open. We've already set up an illogical and hard-to-troubleshoot situation where replication connections do not appear in pg_stat_activity, yet they are counted against max_connections. You could argue that the same is true of superuser_reserved_connections, but there's a couple reasons why it isn't: 1) if superusers are actually connected, that shows up in pg_stat_activity (and given how many of our users run their apps as superuser, they get to max_connections out anyway). 2) the error message spells out that there may be superuser connections available. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 2013-10-15 10:36:41 -0400, Robert Haas wrote: > On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund > wrote: > >> But I also agree that making max_wal_senders act as both a minimum and > >> a maximum is no good. +1 to everything Josh Berkus said. > > > > Josh said we should treat replication connections in a separate "pool" > > from normal database connections, right? So you withdraw your earlier > > objection to that? > > I don't think that's what he said. Here's what I was referring to: > > $ Changing max_wal_senders requires a restart. As such, we currently > $ advise users to set the setting generously: "as many replication > $ connections as you think you'll ever need, plus two". If > $ max_wal_senders is a reservation which could cause the user to run out > $ of other connections sooner than expected, then the user is faced with a > $ new "hard to set" parameter: they don't want to set it too high *or* too > $ low. > $ > $ This would result in a lot of user frustration as they try to get thier > $ connection configuration right and have to restart the server multiple > $ times. I find few new features worth making it *harder* to configure > $ PostgreSQL, and reserved replication connections certainly don't qualify. > $ > $ If it's worth having reserved replication connections (and I can see > $ some reasons to want it), then we need a new GUC for this: > $ "reserved_walsender_connections" I am referring to http://archives.postgresql.org/message-id/525C31D3.3010006%40agliodbs.com : > On 10/14/2013 10:51 AM, Andres Freund wrote: > > Imo the complications around this prove my (way earlier) point that it'd > > be much better to treat replication connections as something entirely > > different to normal SQL connections. There's really not much overlap > > here and while there's some philosophical point to be made about it all > > being connections, from a practical POV treating them separately seems > > better. > > Given that replication connections don't even appear in pg_stat_activity > now, I'd agree with you. I still fail to see what the point is in treating those classes of connections together. It only serves to confuse users and makes considerations way much more complicated. There's no need for reserved replication connections or anything like it if would separate the pools. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund wrote: >> But I also agree that making max_wal_senders act as both a minimum and >> a maximum is no good. +1 to everything Josh Berkus said. > > Josh said we should treat replication connections in a separate "pool" > from normal database connections, right? So you withdraw your earlier > objection to that? I don't think that's what he said. Here's what I was referring to: $ Changing max_wal_senders requires a restart. As such, we currently $ advise users to set the setting generously: "as many replication $ connections as you think you'll ever need, plus two". If $ max_wal_senders is a reservation which could cause the user to run out $ of other connections sooner than expected, then the user is faced with a $ new "hard to set" parameter: they don't want to set it too high *or* too $ low. $ $ This would result in a lot of user frustration as they try to get thier $ connection configuration right and have to restart the server multiple $ times. I find few new features worth making it *harder* to configure $ PostgreSQL, and reserved replication connections certainly don't qualify. $ $ If it's worth having reserved replication connections (and I can see $ some reasons to want it), then we need a new GUC for this: $ "reserved_walsender_connections" -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 2013-10-15 10:29:58 -0400, Robert Haas wrote: > On Tue, Oct 15, 2013 at 12:13 AM, Amit Kapila wrote: > > If we think this way, then may be we should have max_user_connections > > instead of max_connections and then max_wal_connections. But still > > there are other's like pg_basebackup who needs connections and > > tomorrow there can be new such entities which need connection. > > Also we might need to have different infrastructure in code to make > > these options available to users. > > I think having different parameters to configure maximum connections > > for different entities can complicate both code as well as user's job. > > Renaming max_connections is far too big a compatibility break to > consider without far more benefit than what this patch is aiming at. > I'm not prepared to endure the number of beatings I'd have to take if > we did that. +many > But I also agree that making max_wal_senders act as both a minimum and > a maximum is no good. +1 to everything Josh Berkus said. Josh said we should treat replication connections in a separate "pool" from normal database connections, right? So you withdraw your earlier objection to that? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Tue, Oct 15, 2013 at 12:13 AM, Amit Kapila wrote: > If we think this way, then may be we should have max_user_connections > instead of max_connections and then max_wal_connections. But still > there are other's like pg_basebackup who needs connections and > tomorrow there can be new such entities which need connection. > Also we might need to have different infrastructure in code to make > these options available to users. > I think having different parameters to configure maximum connections > for different entities can complicate both code as well as user's job. Renaming max_connections is far too big a compatibility break to consider without far more benefit than what this patch is aiming at. I'm not prepared to endure the number of beatings I'd have to take if we did that. But I also agree that making max_wal_senders act as both a minimum and a maximum is no good. +1 to everything Josh Berkus said. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, Oct 14, 2013 at 11:21 PM, Andres Freund wrote: > On 2013-10-14 10:26:25 -0700, Josh Berkus wrote: >> On 10/13/2013 01:38 AM, Gibheer wrote: >> > So it will ensure that max_wal_senders is used for reserving >> >> connection slots from being used by non-super user connections. I find >> >> new usage of max_wal_senders acceptable, if anyone else thinks >> >> otherwise, please let us know. >> >> I think otherwise. >> >> Changing max_wal_senders requires a restart. As such, we currently >> advise users to set the setting generously: "as many replication >> connections as you think you'll ever need, plus two". If >> max_wal_senders is a reservation which could cause the user to run out >> of other connections sooner than expected, then the user is faced with a >> new "hard to set" parameter: they don't want to set it too high *or* too >> low. >> >> This would result in a lot of user frustration as they try to get thier >> connection configuration right and have to restart the server multiple >> times. I find few new features worth making it *harder* to configure >> PostgreSQL, and reserved replication connections certainly don't qualify. >> >> If it's worth having reserved replication connections (and I can see >> some reasons to want it), then we need a new GUC for this: >> "reserved_walsender_connections" > > Imo the complications around this prove my (way earlier) point that it'd > be much better to treat replication connections as something entirely > different to normal SQL connections. There's really not much overlap > here and while there's some philosophical point to be made about it all > being connections, from a practical POV treating them separately seems > better. > If we were designing on a green field I'd just rename max_connections to > something explicitly talking about client database connections, but > renaming it seems like it'd cause unnecessary pain. If we think this way, then may be we should have max_user_connections instead of max_connections and then max_wal_connections. But still there are other's like pg_basebackup who needs connections and tomorrow there can be new such entities which need connection. Also we might need to have different infrastructure in code to make these options available to users. I think having different parameters to configure maximum connections for different entities can complicate both code as well as user's job. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, Oct 14, 2013 at 10:56 PM, Josh Berkus wrote: > On 10/13/2013 01:38 AM, Gibheer wrote: >> So it will ensure that max_wal_senders is used for reserving >>> connection slots from being used by non-super user connections. I find >>> new usage of max_wal_senders acceptable, if anyone else thinks >>> otherwise, please let us know. > > I think otherwise. > > Changing max_wal_senders requires a restart. As such, we currently > advise users to set the setting generously: "as many replication > connections as you think you'll ever need, plus two". If > max_wal_senders is a reservation which could cause the user to run out > of other connections sooner than expected, then the user is faced with a > new "hard to set" parameter: they don't want to set it too high *or* too > low. It is documented in the patch that configuration max_wal_senders is reserved from max_connections. So it will not cause the user to run out of other connections sooner than expected, if both the variables are configured properly. > This would result in a lot of user frustration as they try to get thier > connection configuration right and have to restart the server multiple > times. I find few new features worth making it *harder* to configure > PostgreSQL, and reserved replication connections certainly don't qualify. > > If it's worth having reserved replication connections (and I can see > some reasons to want it), then we need a new GUC for this: > "reserved_walsender_connections" Having new variable also might make users life difficult, as with new variable he needs to take care of setting 3 parameters (max_wal_senders, reserved_walsender_connections, max_connections) appropriately for this purpose and then choosing value for reserved_walsender_connections will be another thing for which he has to consult. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 10/14/2013 10:51 AM, Andres Freund wrote: > Imo the complications around this prove my (way earlier) point that it'd > be much better to treat replication connections as something entirely > different to normal SQL connections. There's really not much overlap > here and while there's some philosophical point to be made about it all > being connections, from a practical POV treating them separately seems > better. Given that replication connections don't even appear in pg_stat_activity now, I'd agree with you. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 2013-10-14 10:26:25 -0700, Josh Berkus wrote: > On 10/13/2013 01:38 AM, Gibheer wrote: > > So it will ensure that max_wal_senders is used for reserving > >> connection slots from being used by non-super user connections. I find > >> new usage of max_wal_senders acceptable, if anyone else thinks > >> otherwise, please let us know. > > I think otherwise. > > Changing max_wal_senders requires a restart. As such, we currently > advise users to set the setting generously: "as many replication > connections as you think you'll ever need, plus two". If > max_wal_senders is a reservation which could cause the user to run out > of other connections sooner than expected, then the user is faced with a > new "hard to set" parameter: they don't want to set it too high *or* too > low. > > This would result in a lot of user frustration as they try to get thier > connection configuration right and have to restart the server multiple > times. I find few new features worth making it *harder* to configure > PostgreSQL, and reserved replication connections certainly don't qualify. > > If it's worth having reserved replication connections (and I can see > some reasons to want it), then we need a new GUC for this: > "reserved_walsender_connections" Imo the complications around this prove my (way earlier) point that it'd be much better to treat replication connections as something entirely different to normal SQL connections. There's really not much overlap here and while there's some philosophical point to be made about it all being connections, from a practical POV treating them separately seems better. If we were designing on a green field I'd just rename max_connections to something explicitly talking about client database connections, but renaming it seems like it'd cause unnecessary pain. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 10/13/2013 01:38 AM, Gibheer wrote: > So it will ensure that max_wal_senders is used for reserving >> connection slots from being used by non-super user connections. I find >> new usage of max_wal_senders acceptable, if anyone else thinks >> otherwise, please let us know. I think otherwise. Changing max_wal_senders requires a restart. As such, we currently advise users to set the setting generously: "as many replication connections as you think you'll ever need, plus two". If max_wal_senders is a reservation which could cause the user to run out of other connections sooner than expected, then the user is faced with a new "hard to set" parameter: they don't want to set it too high *or* too low. This would result in a lot of user frustration as they try to get thier connection configuration right and have to restart the server multiple times. I find few new features worth making it *harder* to configure PostgreSQL, and reserved replication connections certainly don't qualify. If it's worth having reserved replication connections (and I can see some reasons to want it), then we need a new GUC for this: "reserved_walsender_connections" -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Sun, Oct 13, 2013 at 2:08 PM, Gibheer wrote:
> On Sun, 13 Oct 2013 11:38:17 +0530
> Amit Kapila wrote:
>
>> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
>> wrote:
>> > On Mon, 7 Oct 2013 11:39:55 +0530
>> > Amit Kapila wrote:
>> >> Robert Haas wrote:
>> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> >> 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.
>> >>
>>
>> >
>> > 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.
>>
>> If I understand correctly, now the patch has implementation such that
>> a. if the number of connections left are (ReservedBackends +
>> max_wal_senders), then only superusers or replication connection's
>> will be allowed
>> b. if the number of connections left are ReservedBackend, then only
>> superuser connections will be allowed.
>
> That is correct.
>
>> So it will ensure that max_wal_senders is used for reserving
>> connection slots from being used by non-super user connections. I find
>> new usage of max_wal_senders acceptable, if anyone else thinks
>> otherwise, please let us know.
>>
>>
>> 1.
>> +superuser_reserved_connections
>> +max_wal_senders only superuser and WAL
>> connections
>> +are allowed.
>>
>> Here minus seems to be missing before max_wal_senders and I think it
>> will be better to use replication connections rather than WAL
>> connections.
>
> This is fixed.
>
>> 2.
>> -new replication connections will be accepted.
>> +new WAL or other connections will be accepted.
>>
>> I think as per new implementation, we don't need to change this line.
>
> I reverted that change.
>
>> 3.
>> + * reserved slots from max_connections for wal senders. If the
>> number of free
>> + * slots (max_connections - max_wal_senders) is depleted.
>>
>> Above calculation (max_connections - max_wal_senders) needs to
>> include super user reserved connections.
>
> My first thought was, that I would not add it here. When superuser
> reserved connections are not set, then only max_wal_senders would
> count.
> But you are right, it has to be set, as 3 connections are reserved by
> default for superusers.
+ * slots (max_connections - superuser_reserved_connections - max_wal_senders)
here it should be ReservedBackends rather than superuser_reserved_connections.
>> 4.
>> + /*
>> + * 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")));
>>
>> Will there be any problem if we do the above check before the check
>> for wal senders and reserved replication connections (+
>> !HaveNFreeProcs(max_wal_senders + ReservedBackends))) and don't change
>> the
Re: [HACKERS] Patch for reserved connections for replication users
On Sun, 13 Oct 2013 11:38:17 +0530
Amit Kapila wrote:
> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
> wrote:
> > On Mon, 7 Oct 2013 11:39:55 +0530
> > Amit Kapila wrote:
> >> Robert Haas wrote:
> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> >> 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.
> >>
>
> >
> > 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.
>
> If I understand correctly, now the patch has implementation such that
> a. if the number of connections left are (ReservedBackends +
> max_wal_senders), then only superusers or replication connection's
> will be allowed
> b. if the number of connections left are ReservedBackend, then only
> superuser connections will be allowed.
That is correct.
> So it will ensure that max_wal_senders is used for reserving
> connection slots from being used by non-super user connections. I find
> new usage of max_wal_senders acceptable, if anyone else thinks
> otherwise, please let us know.
>
>
> 1.
> +superuser_reserved_connections
> +max_wal_senders only superuser and WAL
> connections
> +are allowed.
>
> Here minus seems to be missing before max_wal_senders and I think it
> will be better to use replication connections rather than WAL
> connections.
This is fixed.
> 2.
> -new replication connections will be accepted.
> +new WAL or other connections will be accepted.
>
> I think as per new implementation, we don't need to change this line.
I reverted that change.
> 3.
> + * reserved slots from max_connections for wal senders. If the
> number of free
> + * slots (max_connections - max_wal_senders) is depleted.
>
> Above calculation (max_connections - max_wal_senders) needs to
> include super user reserved connections.
My first thought was, that I would not add it here. When superuser
reserved connections are not set, then only max_wal_senders would
count.
But you are right, it has to be set, as 3 connections are reserved by
default for superusers.
> 4.
> + /*
> + * 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")));
>
> Will there be any problem if we do the above check before the check
> for wal senders and reserved replication connections (+
> !HaveNFreeProcs(max_wal_senders + ReservedBackends))) and don't change
> the error message in this check. I think this will ensure that users
> who doesn't enable max_wal_senders will see the same error message as
> before and the purpose to reserve connections for replication can be
> served by your second check.
I have attached two patches, one that checks only max_wal_senders first
and the other checks r
Re: [HACKERS] Patch for reserved connections for replication users
On Thu, Oct 10, 2013 at 3:17 AM, Gibheer wrote:
> On Mon, 7 Oct 2013 11:39:55 +0530
> Amit Kapila wrote:
>> Robert Haas wrote:
>> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> 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.
>>
>
> 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.
If I understand correctly, now the patch has implementation such that
a. if the number of connections left are (ReservedBackends +
max_wal_senders), then only superusers or replication connection's
will be allowed
b. if the number of connections left are ReservedBackend, then only
superuser connections will be allowed.
So it will ensure that max_wal_senders is used for reserving
connection slots from being used by non-super user connections. I find
new usage of max_wal_senders acceptable, if anyone else thinks
otherwise, please let us know.
1.
+superuser_reserved_connections
+max_wal_senders only superuser and WAL connections
+are allowed.
Here minus seems to be missing before max_wal_senders and I think it
will be better to use replication connections rather than WAL
connections.
2.
-new replication connections will be accepted.
+new WAL or other connections will be accepted.
I think as per new implementation, we don't need to change this line.
3.
+ * reserved slots from max_connections for wal senders. If the number of free
+ * slots (max_connections - max_wal_senders) is depleted.
Above calculation (max_connections - max_wal_senders) needs to
include super user reserved connections.
4.
+ /*
+ * 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")));
Will there be any problem if we do the above check before the check
for wal senders and reserved replication connections (+
!HaveNFreeProcs(max_wal_senders + ReservedBackends))) and don't change
the error message in this check. I think this will ensure that users
who doesn't enable max_wal_senders will see the same error message as
before and the purpose to reserve connections for replication can be
served by your second check.
5.
+ gettext_noop("Sets the maximum number wal sender connections and
reserves them."),
Sets the maximum number 'of' wal sender connections and reserves them.
a. 'of' is missing in above line.
b. I think 'reserves them' is not completely right, because super user
connections will be allowed. How about if we add something similar
to 'and reserves them from being used by non-superuser
connections' in above line.
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Fri, 11 Oct 2013 10:00:51 +0530
Amit Kapila wrote:
> On Thu, Oct 10, 2013 at 10:06 PM, Gibheer
> wrote:
> > On Thu, 10 Oct 2013 09:55:24 +0530
> > Amit Kapila wrote:
> >
> >> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
> >> wrote:
> >> > On Mon, 7 Oct 2013 11:39:55 +0530
> >> > Amit Kapila wrote:
> >> >> Robert Haas wrote:
> >> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> >> >> 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.
> >> >>
Re: [HACKERS] Patch for reserved connections for replication users
On Thu, Oct 10, 2013 at 10:06 PM, Gibheer wrote:
> On Thu, 10 Oct 2013 09:55:24 +0530
> Amit Kapila wrote:
>
>> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
>> wrote:
>> > On Mon, 7 Oct 2013 11:39:55 +0530
>> > Amit Kapila wrote:
>> >> Robert Haas wrote:
>> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> >> 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,
>> Ami
Re: [HACKERS] Patch for reserved connections for replication users
On Thu, 10 Oct 2013 09:55:24 +0530
Amit Kapila wrote:
> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
> wrote:
> > On Mon, 7 Oct 2013 11:39:55 +0530
> > Amit Kapila wrote:
> >> Robert Haas wrote:
> >> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> >> 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 pa
Re: [HACKERS] Patch for reserved connections for replication users
I'd received an email from Gibheer suggesting it be move due to lack of time to
work on it. I can certainly move it back if that's no longer the case.
On Oct 9, 2013, at 23:25, Amit Kapila wrote:
> On Thu, Oct 10, 2013 at 3:17 AM, Gibheer wrote:
> On Mon, 7 Oct 2013 11:39:55 +0530
> Amit Kapila wrote:
>> Robert Haas wrote:
>> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Thu, Oct 10, 2013 at 3:17 AM, Gibheer wrote:
> On Mon, 7 Oct 2013 11:39:55 +0530
> Amit Kapila wrote:
>> Robert Haas wrote:
>> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
>> 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila wrote:
> Robert Haas wrote:
> On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
> 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 Radomskidiff --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'
max_connections minus
superuser_reserved_connections, new
connections will be accepted only for superusers, and no
-new replication connections will be accepted.
+new WAL or other connections will be accepted.
@@ -2167,7 +2167,13 @@
Re: [HACKERS] Patch for reserved connections for replication users
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund 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'm also not sure the decision about whether something is a WAL sender is made early enough for the distinction to really make sense. WAL senders actually start off, from the postmaster's POV, as regular backends, and then become walsenders "on the fly". -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On 2013-08-02 08:16:15 -0400, Robert Haas wrote: > On Tue, Jul 30, 2013 at 3:10 AM, Gibheer wrote: > > here is an update off my patch based on the discussion with Marko > > Tiikkaja and Andres Freund. > > > > Marko and I had the idea of introducing reserved connections based on > > roles as it would create a way to garantuee specific roles to connect > > when other roles use up all connections for whatever reason. But > > Andreas said, that it would make connecting take much too long. > > > > So to just fix the issue at hand, we decided that adding > > max_wal_senders to the pool of reserved connections is better. With > > that, we are sure that streaming replication can connect to the master. > > > > So instead of creating a new configuration option I added > > max_wal_senders to the reserved connections and changed the check for > > new connections. > > > > The test.pl is a small script to test, if the patch does what it should. > > 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. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Fri, 2 Aug 2013 08:16:15 -0400 Robert Haas wrote: > On Tue, Jul 30, 2013 at 3:10 AM, Gibheer > wrote: > > here is an update off my patch based on the discussion with Marko > > Tiikkaja and Andres Freund. > > > > Marko and I had the idea of introducing reserved connections based > > on roles as it would create a way to garantuee specific roles to > > connect when other roles use up all connections for whatever > > reason. But Andreas said, that it would make connecting take much > > too long. > > > > So to just fix the issue at hand, we decided that adding > > max_wal_senders to the pool of reserved connections is better. With > > that, we are sure that streaming replication can connect to the > > master. > > > > So instead of creating a new configuration option I added > > max_wal_senders to the reserved connections and changed the check > > for new connections. > > > > The test.pl is a small script to test, if the patch does what it > > should. > > 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()). > You are right, that can't be correct. The slots I added with max_wal_sender would end up as background worker slots. I have to check my tests again. In my first patch I just copied the part to limit the connections based on superuser reserved connections + replication reserved connections. That did not change the definition of max_connections and made superuser connections higher in priority than replication connections. Is that the better approach? Thank you for your input. Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
On Tue, Jul 30, 2013 at 3:10 AM, Gibheer wrote: > here is an update off my patch based on the discussion with Marko > Tiikkaja and Andres Freund. > > Marko and I had the idea of introducing reserved connections based on > roles as it would create a way to garantuee specific roles to connect > when other roles use up all connections for whatever reason. But > Andreas said, that it would make connecting take much too long. > > So to just fix the issue at hand, we decided that adding > max_wal_senders to the pool of reserved connections is better. With > that, we are sure that streaming replication can connect to the master. > > So instead of creating a new configuration option I added > max_wal_senders to the reserved connections and changed the check for > new connections. > > The test.pl is a small script to test, if the patch does what it should. 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()). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for reserved connections for replication users
Hi, here is an update off my patch based on the discussion with Marko Tiikkaja and Andres Freund. Marko and I had the idea of introducing reserved connections based on roles as it would create a way to garantuee specific roles to connect when other roles use up all connections for whatever reason. But Andreas said, that it would make connecting take much too long. So to just fix the issue at hand, we decided that adding max_wal_senders to the pool of reserved connections is better. With that, we are sure that streaming replication can connect to the master. So instead of creating a new configuration option I added max_wal_senders to the reserved connections and changed the check for new connections. The test.pl is a small script to test, if the patch does what it should. regards, Stefan Radomskidiff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 23ebc11..2ba98e2 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2170,8 +2170,10 @@ include 'filename' processes). 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 -. This parameter can only -be set at server start. wal_level must be set +. Like + this option reserves +connections from . This parameter +can only be set at server start. wal_level must be set to archive or hot_standby to allow connections from standby servers. diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 2c7f0f1..3194894 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -436,7 +436,7 @@ InitializeMaxBackends(void) /* the extra unit accounts for the autovacuum launcher */ MaxBackends = MaxConnections + autovacuum_max_workers + 1 + - + max_worker_processes; + + max_worker_processes + max_wal_senders; /* internal error because the values were all checked previously */ if (MaxBackends > MAX_BACKENDS) @@ -705,7 +705,7 @@ InitPostgres(const char *in_dbname, Oid dboid, const char *username, * don't allow them to consume the reserved slots, which are intended for * interactive use. */ - if ((!am_superuser || am_walsender) && + if ((!am_superuser && !am_walsender) && ReservedBackends > 0 && !HaveNFreeProcs(ReservedBackends)) ereport(FATAL, test.pl Description: Perl program -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
