Fwd: Patch for reserved connections for replication users

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no longer
mean the maximum number of connections, but rather the maximum
number of non-replication connections. I don't think I support
that definitional change, and I'm kinda surprised if this is
sufficient to implement it anyway (e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.

Because replication connections are still "connections". If I tell
the system I want to allow 100 connections to the server, it should
allow 100 connections, not 110 or 95 or any other number.

I think that to reserve connections for replication, mechanism similar
to superuser_reserved_connections be used rather than auto vacuum
workers or background workers.
This won't change the definition of MaxConnections. Another thing is
that rather than introducing new parameter for replication reserved
connections, it is better to use max_wal_senders as it can serve the
purpose.

Review for replication_reserved_connections-v2.patch, considering we
are going to use mechanism similar to superuser_reserved_connections
and won't allow definition of MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication and
superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running WAL
sender processes."),

4. With this approach, there is no need to change InitProcGlobal(), as
it is used to keep track bgworkerFreeProcs and autovacFreeProcs, for
others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of max_wal_enders
in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new connections
will be accepted only for superusers, and no new replication
connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only superuser
reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for superuser
use.
* This number is taken out of the pool size given by MaxBackends so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really means is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser connections
don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for max_wal_senders
similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!

So this patch uses max_wal_senders together with the idea of the first
patch I sent. The error messages are also adjusted to make it obvious,
how it is supposed to be and all checks work, as far as I could tell.

regards,

Stefan Radomski

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org> wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no longer
mean the maximum number of connections, but rather the maximum
number of non-replication connections. I don't think I support
that definitional change, and I'm kinda surprised if this is
sufficient to implement it anyway (e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.

Because replication connections are still "connections". If I tell
the system I want to allow 100 connections to the server, it should
allow 100 connections, not 110 or 95 or any other number.

I think that to reserve connections for replication, mechanism similar
to superuser_reserved_connections be used rather than auto vacuum
workers or background workers.
This won't change the definition of MaxConnections. Another thing is
that rather than introducing new parameter for replication reserved
connections, it is better to use max_wal_senders as it can serve the
purpose.

Review for replication_reserved_connections-v2.patch, considering we
are going to use mechanism similar to superuser_reserved_connections
and won't allow definition of MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication and
superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running WAL
sender processes."),

4. With this approach, there is no need to change InitProcGlobal(), as
it is used to keep track bgworkerFreeProcs and autovacFreeProcs, for
others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of max_wal_enders
in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new connections
will be accepted only for superusers, and no new replication
connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only superuser
reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for superuser
use.
* This number is taken out of the pool size given by MaxBackends so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really means is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser connections
don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for max_wal_senders
similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!

Is there any specific reason why you moved this patch to next CommitFest?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

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 <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org> wrote:
On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no longer
mean the maximum number of connections, but rather the maximum
number of non-replication connections. I don't think I support
that definitional change, and I'm kinda surprised if this is
sufficient to implement it anyway (e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.

Because replication connections are still "connections". If I tell
the system I want to allow 100 connections to the server, it should
allow 100 connections, not 110 or 95 or any other number.

I think that to reserve connections for replication, mechanism similar
to superuser_reserved_connections be used rather than auto vacuum
workers or background workers.
This won't change the definition of MaxConnections. Another thing is
that rather than introducing new parameter for replication reserved
connections, it is better to use max_wal_senders as it can serve the
purpose.

Review for replication_reserved_connections-v2.patch, considering we
are going to use mechanism similar to superuser_reserved_connections
and won't allow definition of MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication and
superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running WAL
sender processes."),

4. With this approach, there is no need to change InitProcGlobal(), as
it is used to keep track bgworkerFreeProcs and autovacFreeProcs, for
others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of max_wal_enders
in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new connections
will be accepted only for superusers, and no new replication
connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only superuser
reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for superuser
use.
* This number is taken out of the pool size given by MaxBackends so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really means is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser connections
don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for max_wal_senders
similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!

Is there any specific reason why you moved this patch to next CommitFest?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, 10 Oct 2013 09:55:24 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org>
wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather the
maximum number of non-replication connections. I don't think
I support that definitional change, and I'm kinda surprised if
this is sufficient to implement it anyway (e.g. see
InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make
sense. That they end up allocating similar internal data seems
to be an implementation detail to me.

Because replication connections are still "connections". If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.

I think that to reserve connections for replication, mechanism
similar to superuser_reserved_connections be used rather than auto
vacuum workers or background workers.
This won't change the definition of MaxConnections. Another thing
is that rather than introducing new parameter for replication
reserved connections, it is better to use max_wal_senders as it
can serve the purpose.

Review for replication_reserved_connections-v2.patch, considering
we are going to use mechanism similar to
superuser_reserved_connections and won't allow definition of
MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication and
superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running WAL
sender processes."),

4. With this approach, there is no need to change
InitProcGlobal(), as it is used to keep track bgworkerFreeProcs
and autovacFreeProcs, for others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of
max_wal_enders in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new
connections will be accepted only for superusers, and no new
replication connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only superuser
reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for
superuser use.
* This number is taken out of the pool size given by MaxBackends
so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really means
is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser connections
don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for max_wal_senders
similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!

Is there any specific reason why you moved this patch to next
CommitFest?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Mike asked me about the status of the patch a couple days back and I
didn't think I would be able to work on the patch so soon again. That's
why I told him to just move the patch into the next commitfest.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Oct 10, 2013 at 10:06 PM, Gibheer <gibheer@zero-knowledge.org> wrote:

On Thu, 10 Oct 2013 09:55:24 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org>
wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather the
maximum number of non-replication connections. I don't think
I support that definitional change, and I'm kinda surprised if
this is sufficient to implement it anyway (e.g. see
InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make
sense. That they end up allocating similar internal data seems
to be an implementation detail to me.

Because replication connections are still "connections". If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.

I think that to reserve connections for replication, mechanism
similar to superuser_reserved_connections be used rather than auto
vacuum workers or background workers.
This won't change the definition of MaxConnections. Another thing
is that rather than introducing new parameter for replication
reserved connections, it is better to use max_wal_senders as it
can serve the purpose.

Review for replication_reserved_connections-v2.patch, considering
we are going to use mechanism similar to
superuser_reserved_connections and won't allow definition of
MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication and
superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running WAL
sender processes."),

4. With this approach, there is no need to change
InitProcGlobal(), as it is used to keep track bgworkerFreeProcs
and autovacFreeProcs, for others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of
max_wal_enders in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new
connections will be accepted only for superusers, and no new
replication connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only superuser
reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for
superuser use.
* This number is taken out of the pool size given by MaxBackends
so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really means
is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser connections
don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for max_wal_senders
similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till now.
Thank you very much Amit!

Is there any specific reason why you moved this patch to next
CommitFest?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Mike asked me about the status of the patch a couple days back and I
didn't think I would be able to work on the patch so soon again. That's
why I told him to just move the patch into the next commitfest.

But you have already posted patch after fixing comments and I am
planning to review again on coming weekend; if every thing is okay,
then there is a chance that you can get committer level feedback for
your patch in this CF. In the end it's upto you to decide.

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

On Fri, 11 Oct 2013 10:00:51 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 10:06 PM, Gibheer
<gibheer@zero-knowledge.org> wrote:

On Thu, 10 Oct 2013 09:55:24 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer
<gibheer@zero-knowledge.org> wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather
the maximum number of non-replication connections. I don't
think I support that definitional change, and I'm kinda
surprised if this is sufficient to implement it anyway
(e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't
you like the definitional change? The set of things you can
do from replication connections are completely different
from a normal connection. So using separate "pools" for them
seems to make sense. That they end up allocating similar
internal data seems to be an implementation detail to me.

Because replication connections are still "connections". If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.

I think that to reserve connections for replication, mechanism
similar to superuser_reserved_connections be used rather than
auto vacuum workers or background workers.
This won't change the definition of MaxConnections. Another
thing is that rather than introducing new parameter for
replication reserved connections, it is better to use
max_wal_senders as it can serve the purpose.

Review for replication_reserved_connections-v2.patch,
considering we are going to use mechanism similar to
superuser_reserved_connections and won't allow definition of
MaxConnections to change.

1. /* the extra unit accounts for the autovacuum launcher */
MaxBackends = MaxConnections + autovacuum_max_workers + 1 +
- + max_worker_processes;
+ + max_worker_processes + max_wal_senders;

Above changes are not required.

2.
+ if ((!am_superuser && !am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Change the check as you have in patch-1 for
ReserveReplicationConnections

if (!am_superuser &&
(max_wal_senders > 0 || ReservedBackends > 0) &&
!HaveNFreeProcs(max_wal_senders + ReservedBackends))
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("remaining connection slots are reserved for replication
and superuser connections")));

3. In guc.c, change description of max_wal_senders similar to
superuser_reserved_connections at below place:
{"max_wal_senders", PGC_POSTMASTER, REPLICATION_SENDING,
gettext_noop("Sets the maximum number of simultaneously running
WAL sender processes."),

4. With this approach, there is no need to change
InitProcGlobal(), as it is used to keep track bgworkerFreeProcs
and autovacFreeProcs, for others it use freeProcs.

5. Below description in config.sgml needs to be changed for
superuser_reserved_connections to include the effect of
max_wal_enders in reserved connections.
Whenever the number of active concurrent connections is at least
max_connections minus superuser_reserved_connections, new
connections will be accepted only for superusers, and no new
replication connections will be accepted.

6. Also similar description should be added to max_wal_senders
configuration parameter.

7. Below comment needs to be updated, as it assumes only
superuser reserved connections as part of MaxConnections limit.
/*
* ReservedBackends is the number of backends reserved for
superuser use.
* This number is taken out of the pool size given by
MaxBackends so
* number of backend slots available to non-superusers is
* (MaxBackends - ReservedBackends). Note what this really
means is
* "if there are <= ReservedBackends connections available, only
superusers
* can make new connections" --- pre-existing superuser
connections don't
* count against the limit.
*/
int ReservedBackends;

8. Also we can add comment on top of definition for
max_wal_senders similar to ReservedBackends.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Hi,

I took the time and reworked the patch with the feedback till
now. Thank you very much Amit!

Is there any specific reason why you moved this patch to next
CommitFest?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Mike asked me about the status of the patch a couple days back and I
didn't think I would be able to work on the patch so soon again.
That's why I told him to just move the patch into the next
commitfest.

But you have already posted patch after fixing comments and I am
planning to review again on coming weekend; if every thing is okay,
then there is a chance that you can get committer level feedback for
your patch in this CF. In the end it's upto you to decide.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

You are right. If we can get it working in this commit fest, then it is
out of the way for the next.

@Mike: Can you move the patch back to this commit fest? Amit sounds
like we can get this done, so I will support that and work as hard to
make it happen.

And thank you for all your feedback.

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

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org> wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no longer
mean the maximum number of connections, but rather the maximum
number of non-replication connections. I don't think I support
that definitional change, and I'm kinda surprised if this is
sufficient to implement it anyway (e.g. see InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make sense.
That they end up allocating similar internal data seems to be an
implementation detail to me.

Because replication connections are still "connections". If I tell
the system I want to allow 100 connections to the server, it should
allow 100 connections, not 110 or 95 or any other number.

I think that to reserve connections for replication, mechanism similar
to superuser_reserved_connections be used rather than auto vacuum
workers or background workers.
This won't change the definition of MaxConnections. Another thing is
that rather than introducing new parameter for replication reserved
connections, it is better to use max_wal_senders as it can serve the
purpose.

Review for replication_reserved_connections-v2.patch, considering we
are going to use mechanism similar to superuser_reserved_connections
and won't allow definition of MaxConnections to change.

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.
+        <varname>superuser_reserved_connections</varname>
+        <varname>max_wal_senders</varname> 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

On Sun, 13 Oct 2013 11:38:17 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org>
wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather the
maximum number of non-replication connections. I don't think
I support that definitional change, and I'm kinda surprised if
this is sufficient to implement it anyway (e.g. see
InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make
sense. That they end up allocating similar internal data seems
to be an implementation detail to me.

Because replication connections are still "connections". If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.

I think that to reserve connections for replication, mechanism
similar to superuser_reserved_connections be used rather than auto
vacuum workers or background workers.
This won't change the definition of MaxConnections. Another thing
is that rather than introducing new parameter for replication
reserved connections, it is better to use max_wal_senders as it
can serve the purpose.

Review for replication_reserved_connections-v2.patch, considering
we are going to use mechanism similar to
superuser_reserved_connections and won't allow definition of
MaxConnections to change.

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.
+        <varname>superuser_reserved_connections</varname>
+        <varname>max_wal_senders</varname> 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 reserved_backends first. Both return the original
message, when max_wal_sender is not set and I think it is only a matter
of taste, which comes first.
Me, I would prefer max_wal_senders to get from more connections to less.

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.

This is fixed.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Thank you again for your feedback.

regards,

Stefan Radomski

On Sun, Oct 13, 2013 at 2:08 PM, Gibheer <gibheer@zero-knowledge.org> wrote:

On Sun, 13 Oct 2013 11:38:17 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

On Thu, Oct 10, 2013 at 3:17 AM, Gibheer <gibheer@zero-knowledge.org>
wrote:

On Mon, 7 Oct 2013 11:39:55 +0530
Amit Kapila <amit.kapila16@gmail.com> wrote:

Robert Haas wrote:
On Mon, Aug 5, 2013 at 2:04 AM, Andres Freund
<andres(at)2ndquadrant(dot)com> wrote:

Hmm. It seems like this match is making MaxConnections no
longer mean the maximum number of connections, but rather the
maximum number of non-replication connections. I don't think
I support that definitional change, and I'm kinda surprised if
this is sufficient to implement it anyway (e.g. see
InitProcGlobal()).

I don't think the implementation is correct, but why don't you
like the definitional change? The set of things you can do from
replication connections are completely different from a normal
connection. So using separate "pools" for them seems to make
sense. That they end up allocating similar internal data seems
to be an implementation detail to me.

Because replication connections are still "connections". If I
tell the system I want to allow 100 connections to the server,
it should allow 100 connections, not 110 or 95 or any other
number.

I think that to reserve connections for replication, mechanism
similar to superuser_reserved_connections be used rather than auto
vacuum workers or background workers.
This won't change the definition of MaxConnections. Another thing
is that rather than introducing new parameter for replication
reserved connections, it is better to use max_wal_senders as it
can serve the purpose.

Review for replication_reserved_connections-v2.patch, considering
we are going to use mechanism similar to
superuser_reserved_connections and won't allow definition of
MaxConnections to change.

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.
+        <varname>superuser_reserved_connections</varname>
+        <varname>max_wal_senders</varname> 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 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 reserved_backends first. Both return the original
message, when max_wal_sender is not set and I think it is only a matter
of taste, which comes first.
Me, I would prefer max_wal_senders to get from more connections to less.

I think there is no major problem even if we keep max_wal_senders
check first, but there could be error message inconsistency. Please
consider below scenario:
max_connections=5
superuser_reserved_connections=3
max_wal_senders = 2

1. Start primary database server M1
2. Start first standby S1
3. Start second standby S2
4. Now try to connect by non-super user to M1 -- here it will return
error msg as "psql: FATAL: remaining connection slots are reserved
for replication
and superuser connections"
By above error message, it seems that replication connection is
allowed, but actually it will give error for new replication
connection, see next step
5. Start third standby S3 -- here an appropriate error message "FATAL:
could not connect to the primary server: FATAL: remaining connection
slots are reserved for non-replication superuser connections"

I feel there is minor inconsistency in step-4 if we use
max_wal_senders check before ReservedBackends 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.

This is fixed.

+ gettext_noop("Sets the maximum number of wal sender connections and reserves "
+ "them from being used non-superuser connections."),

"them from being used 'by' non-superuser connections."
'by' is missing in above line.

1.
if (ReservedBackends >= MaxConnections)
{
write_stderr("%s: superuser_reserved_connections must be less than
max_connections\n", progname);
ExitPostmaster(1);
}

I think we should have one check similar to above for (max_wal_senders
+ ReservedBackends >= MaxConnections) to avoid server starting
with values, where not even 1 non-superuser connection will be
allowed. I think this is a reasoning similar to why the check for
ReservedBackends exists.

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

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

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

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

On Mon, Oct 14, 2013 at 10:56 PM, Josh Berkus <josh@agliodbs.com> 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

On Mon, Oct 14, 2013 at 11:21 PM, Andres Freund <andres@2ndquadrant.com> 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

On Tue, Oct 15, 2013 at 12:13 AM, Amit Kapila <amit.kapila16@gmail.com> 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

On 2013-10-15 10:29:58 -0400, Robert Haas wrote:

On Tue, Oct 15, 2013 at 12:13 AM, Amit Kapila <amit.kapila16@gmail.com> 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

On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund <andres@2ndquadrant.com> 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

On 2013-10-15 10:36:41 -0400, Robert Haas wrote:

On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund <andres@2ndquadrant.com> 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

On 10/15/2013 07:36 AM, Robert Haas wrote:

On Tue, Oct 15, 2013 at 10:34 AM, Andres Freund <andres@2ndquadrant.com> 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