Patch for reserved connections for replication users

Hi,

this patch introduces a new configuration flag
replication_reserved_connections to reserve connection slots for
replication in the same way superuser_reserved_connections works for
superusers.

This helps in cases where the application opens connections until
max_connections is reached. A slave would not be able to connect to the
master now and would just be down. With this patch the slave is able to
connect.

This option does not influence the superuser_reserved_connections, so
new slaves are not able to open new connections when the reserved
replication slots are filled.

The only thing this patch is missing are tests. Where should I put them
into the source tree?

thank you,

Stefan Radomski

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 Radomski

On Tue, Jul 30, 2013 at 3:10 AM, Gibheer <gibheer(at)zero-knowledge(dot)org> 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

On Fri, 2 Aug 2013 08:16:15 -0400
Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Tue, Jul 30, 2013 at 3:10 AM, Gibheer <gibheer(at)zero-knowledge(dot)org>
> 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

On 2013-08-02 08:16:15 -0400, Robert Haas wrote:
> On Tue, Jul 30, 2013 at 3:10 AM, Gibheer <gibheer(at)zero-knowledge(dot)org> 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

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'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