Streaming replication document improvements

Fujii Masao wrote:

***************
*** 829,834 **** if (!triggered)
--- 826,834 ----
<para>
Set the maximum number of concurrent connections from the standby servers
(see <xref linkend="guc-max-wal-senders"> for details).
+        Since those connections are for superusers,
+        <xref linkend="guc-superuser-reserved-connections"> should be
+        set accordingly.
</para>
</listitem>
<listitem>

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots? How should
superuser_reserved_connections be set?

*** a/src/backend/access/transam/recovery.conf.sample
--- b/src/backend/access/transam/recovery.conf.sample
***************
*** 88,94 ****
#recovery_target_timeline = '33'		# number or 'latest'
#
#---------------------------------------------------------------------------
! # LOG-STREAMING REPLICATION PARAMETERS
#---------------------------------------------------------------------------
#
# When standby_mode is enabled, the PostgreSQL server will work as
--- 88,94 ----
#recovery_target_timeline = '33'		# number or 'latest'
#
#---------------------------------------------------------------------------
! # STANDBY SERVER PARAMETERS
#---------------------------------------------------------------------------
#
# When standby_mode is enabled, the PostgreSQL server will work as

Hmm, that makes the HOT STANDBY notice after that section look weird:

#---------------------------------------------------------------------------
# HOT STANDBY PARAMETERS
#---------------------------------------------------------------------------
#
# Hot Standby related parameters are listed in postgresql.conf
#
#---------------------------------------------------------------------------

Do we need that notice? Maybe just move that sentence to the "STANDBY
SERVER PARAMETERS" section.

I just committed a patch to move around the high-availability sections a
bit. That caused conflicts with this patch, so I picked the changes from
the patch and applied them over the new layout, and I also did a lot of
other editing. So, I committed most parts of this patch (except the
above), with a lot of changes to fix the bit-rot, and also other editing
to my liking. I hope I made it better not worse.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

On Thu, Apr 1, 2010 at 5:39 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Fujii Masao wrote:

***************
*** 829,834 **** if (!triggered)
--- 826,834 ----
        <para>
         Set the maximum number of concurrent connections from the standby servers
         (see <xref linkend="guc-max-wal-senders"> for details).
+        Since those connections are for superusers,
+        <xref linkend="guc-superuser-reserved-connections"> should be
+        set accordingly.
        </para>
       </listitem>
       <listitem>

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots?

Yes. Since SR connection is superuser connection, setting
superuser_reserved_connections appropriately would be useful
to prevent non-superuser connections from blocking the connection
from the standby.

How should
superuser_reserved_connections be set?

Well.. setting the number of superuser connection required for
maintenance plus max_wal_senders seems to be reasonable.

*** a/src/backend/access/transam/recovery.conf.sample
--- b/src/backend/access/transam/recovery.conf.sample
***************
*** 88,94 ****
  #recovery_target_timeline = '33'            # number or 'latest'
  #
  #---------------------------------------------------------------------------
! # LOG-STREAMING REPLICATION PARAMETERS
  #---------------------------------------------------------------------------
  #
  # When standby_mode is enabled, the PostgreSQL server will work as
--- 88,94 ----
  #recovery_target_timeline = '33'            # number or 'latest'
  #
  #---------------------------------------------------------------------------
! # STANDBY SERVER PARAMETERS
  #---------------------------------------------------------------------------
  #
  # When standby_mode is enabled, the PostgreSQL server will work as

Hmm, that makes the HOT STANDBY notice after that section look weird:

#---------------------------------------------------------------------------
# HOT STANDBY PARAMETERS
#---------------------------------------------------------------------------
#
# Hot Standby related parameters are listed in postgresql.conf
#
#---------------------------------------------------------------------------

Do we need that notice? Maybe just move that sentence to the "STANDBY
SERVER PARAMETERS" section.

I don't think that the notice is necessary. But I agree to the move.

I just committed a patch to move around the high-availability sections a
bit. That caused conflicts with this patch, so I picked the changes from
the patch and applied them over the new layout, and I also did a lot of
other editing. So, I committed most parts of this patch (except the
above), with a lot of changes to fix the bit-rot, and also other editing
to my liking. I hope I made it better not worse.

Thanks a lot!

doc/src/sgml/monitoring.sgml

+    In streaming replication (see <xref linkend="streaming-replication"> for details),
+    WAL sender process is listed in the <command>ps</> display on the primary server.
+    The form of its command line display is:
+ <screen>
+ postgres: wal sender process <replaceable>user</> <replaceable>host</> streaming <replaceable>location</>
+ </screen>
+    The user and host items remain the same for the life of the replication connection.
+    The location indicates how far WAL sender process has sent the WAL.
+    On the other hand, WAL receiver process is listed in the <command>ps</> display
+    on the standby server. The form of its command line display is:
+ <screen>
+ postgres: wal receiver process streaming <replaceable>location</>
+ </screen>
+    The location indicates how far WAL receiver has received the WAL.
+   </para>
+
+   <para>

The original patch includes the above change for monitoring.sgml, but
it's been excluded from the commit. You think that it's not worth applying
the change?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Wed, Mar 31, 2010 at 9:50 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots?

Yes. Since SR connection is superuser connection, setting
superuser_reserved_connections appropriately would be useful
to prevent non-superuser connections from blocking the connection
from the standby.

I think this is wacky, especially since we'd someday like to have
replication be a separate privilege from superuser. I think we should
change this.

...Robert

On Thu, Apr 1, 2010 at 10:52 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:50 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots?

Yes. Since SR connection is superuser connection, setting
superuser_reserved_connections appropriately would be useful
to prevent non-superuser connections from blocking the connection
from the standby.

I think this is wacky, especially since we'd someday like to have
replication be a separate privilege from superuser.  I think we should
change this.

You mean that we should change replication connection not to consume
superuser_reserved_connections slots in 9.0?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Wed, Mar 31, 2010 at 9:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Apr 1, 2010 at 10:52 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:50 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots?

Yes. Since SR connection is superuser connection, setting
superuser_reserved_connections appropriately would be useful
to prevent non-superuser connections from blocking the connection
from the standby.

I think this is wacky, especially since we'd someday like to have
replication be a separate privilege from superuser.  I think we should
change this.

You mean that we should change replication connection not to consume
superuser_reserved_connections slots in 9.0?

Yes.

...Robert

On Thu, Apr 1, 2010 at 11:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Apr 1, 2010 at 10:52 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:50 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

That's an interesting point, do streaming replication connections
consume superuser_reserved_connections slots?

Yes. Since SR connection is superuser connection, setting
superuser_reserved_connections appropriately would be useful
to prevent non-superuser connections from blocking the connection
from the standby.

I think this is wacky, especially since we'd someday like to have
replication be a separate privilege from superuser.  I think we should
change this.

You mean that we should change replication connection not to consume
superuser_reserved_connections slots in 9.0?

Yes.

Preventing superuser connections from consuming superuser_reserved_connections
slots seems strange for me. So I'm leaning toward just removing superuser
privilege from replication connection again. Thought?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Fujii Masao wrote:

On Thu, Apr 1, 2010 at 11:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

You mean that we should change replication connection not to consume
superuser_reserved_connections slots in 9.0?

Yes.

I think it's good that walsenders can use the superuser reserved slots,
that way a client that opens max_connections connections can't block out
standby servers from connecting.

Preventing superuser connections from consuming superuser_reserved_connections
slots seems strange for me. So I'm leaning toward just removing superuser
privilege from replication connection again. Thought?

That would be good, but I fear it's a bigger change than we should be
doing at this point.

How about we adjust the backends math a bit:

Currently:

ReservedBackends = superuser_reserved_connections
MaxBackends = max_connections + autovacuum_max_workers + 1;

Proposal:

ReservedBackends = superuser_reserved_connections + max_wal_senders
MaxBackends = max_connections + autovacuum_max_workers + max_wal_senders + 1

So we implicitly reserve a slot and a superuser reserved slot for each
walsender. Walsenders use the slots reserved for superusers, but if you
set superuser_reserved_connections=3, there's still always at least
three slots available for superuser to log in with psql, even if the
maximum number of walsenders are connected.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:

Fujii Masao wrote:

Preventing superuser connections from consuming superuser_reserved_connections
slots seems strange for me. So I'm leaning toward just removing superuser
privilege from replication connection again. Thought?

That would be good, but I fear it's a bigger change than we should be
doing at this point.

Exactly. Despite Robert's unhappiness, NONE of this should get changed
right now. When and if we create a separate replication privilege,
it'll be time enough to revisit the connection limit arithmetic.

regards, tom lane

On Thu, Apr 1, 2010 at 9:09 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:

Fujii Masao wrote:

On Thu, Apr 1, 2010 at 11:00 AM, Robert Haas <robertmhaas@gmail.com> wrote:

On Wed, Mar 31, 2010 at 9:58 PM, Fujii Masao <masao.fujii@gmail.com> wrote:

You mean that we should change replication connection not to consume
superuser_reserved_connections slots in 9.0?

Yes.

I think it's good that walsenders can use the superuser reserved slots,
that way a client that opens max_connections connections can't block out
standby servers from connecting.

Preventing superuser connections from consuming superuser_reserved_connections
slots seems strange for me. So I'm leaning toward just removing superuser
privilege from replication connection again. Thought?

That would be good, but I fear it's a bigger change than we should be
doing at this point.

How about we adjust the backends math a bit:

Currently:

ReservedBackends = superuser_reserved_connections
MaxBackends = max_connections + autovacuum_max_workers + 1;

Proposal:

ReservedBackends = superuser_reserved_connections + max_wal_senders
MaxBackends = max_connections + autovacuum_max_workers + max_wal_senders + 1

So we implicitly reserve a slot and a superuser reserved slot for each
walsender. Walsenders use the slots reserved for superusers, but if you
set superuser_reserved_connections=3, there's still always at least
three slots available for superuser to log in with psql, even if the
maximum number of walsenders are connected.

That seems pretty reasonable to me. I haven't checked how much code
impact there is. I know Tom doesn't think we should change it at all,
but surely pre-beta is the time to fix nasty corner cases that were
added by recently committed patches?

...Robert

Robert Haas <robertmhaas@gmail.com> writes:

That seems pretty reasonable to me. I haven't checked how much code
impact there is. I know Tom doesn't think we should change it at all,
but surely pre-beta is the time to fix nasty corner cases that were
added by recently committed patches?

What nasty corner case? Having replication connections use superuser
reserved slots seems exactly the behavior I'd expect given that they are
running as superuser. I agree it would be good to decouple that later,
but we already decided we are not going to try to separate replication
privilege from superuser in 9.0.

(Also, autovacuum workers are a quite separate concept since the DBA
doesn't set them up or deal with them directly. So I'm unimpressed by
pointing to the treatment of autovacuum_max_workers as a precedent.)

regards, tom lane

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Having replication connections use superuser reserved slots seems
exactly the behavior I'd expect given that they are running as
superuser.

It seems within the realm of possibility that not all users would
think to boost superuser_reserved_connections by the number of
replication connections, and be surprised when they are unable to
connect in an emergency.

-Kevin

On 4/1/10 10:44 AM, Kevin Grittner wrote:

It seems within the realm of possibility that not all users would
think to boost superuser_reserved_connections by the number of
replication connections, and be surprised when they are unable to
connect in an emergency.

Well, that's easy to add to the documentation.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

On Thu, Apr 1, 2010 at 1:46 PM, Josh Berkus <josh@agliodbs.com> wrote:

On 4/1/10 10:44 AM, Kevin Grittner wrote:

It seems within the realm of possibility that not all users would
think to boost superuser_reserved_connections by the number of
replication connections, and be surprised when they are unable to
connect in an emergency.

Well, that's easy to add to the documentation.

It's probably also easy to fix so that it doesn't NEED to be documented.

The thing is, when dealing with new features, we reduce our overall
maintenance burden if we get it right the first time. Obviously it's
too late for major changes, but minor adjustments to maintain the POLA
seem like exactly what we SHOULD be doing right now.

...Robert

The thing is, when dealing with new features, we reduce our overall
maintenance burden if we get it right the first time. Obviously it's
too late for major changes, but minor adjustments to maintain the POLA
seem like exactly what we SHOULD be doing right now.

Oh, I agree. Since we have a separate WALSender limit, it seems
counter-intuitive and difficult-to-admin to have the WALSenders also
limited by superuser_connections. They should be their own separate
connection pool, just like the other "background" processes.

However, if this was somehow infeasible, it wouldn't be hard to
document. That's all.

--
-- Josh Berkus
PostgreSQL Experts Inc.
http://www.pgexperts.com

Josh Berkus <josh@agliodbs.com> writes:

Oh, I agree. Since we have a separate WALSender limit, it seems
counter-intuitive and difficult-to-admin to have the WALSenders also
limited by superuser_connections. They should be their own separate
connection pool, just like the other "background" processes.

+1

Regards,
--
dim

On Thu, 2010-04-01 at 11:49 -0700, Josh Berkus wrote:

The thing is, when dealing with new features, we reduce our overall
maintenance burden if we get it right the first time. Obviously it's
too late for major changes, but minor adjustments to maintain the POLA
seem like exactly what we SHOULD be doing right now.

Oh, I agree. Since we have a separate WALSender limit, it seems
counter-intuitive and difficult-to-admin to have the WALSenders also
limited by superuser_connections. They should be their own separate
connection pool, just like the other "background" processes.

However, if this was somehow infeasible, it wouldn't be hard to
document. That's all.

+1

--
Simon Riggs www.2ndQuadrant.com

On Fri, Apr 2, 2010 at 2:58 AM, Robert Haas <robertmhaas@gmail.com> wrote:

It's probably also easy to fix so that it doesn't NEED to be documented.

The thing is, when dealing with new features, we reduce our overall
maintenance burden if we get it right the first time.  Obviously it's
too late for major changes, but minor adjustments to maintain the POLA
seem like exactly what we SHOULD be doing right now.

The attached patch implements the Heikki's proposal:

----------
ReservedBackends = superuser_reserved_connections + max_wal_senders
MaxBackends = max_connections + autovacuum_max_workers + max_wal_senders + 1
----------

This change looks like minor adjustments rather than major changes.

I have one question:
In the patch, max_wal_senders is ignored by CheckRequiredParameterValues()
as autovacuum_max_workers is already. Is this OK for HS?

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

On Fri, Apr 2, 2010 at 2:06 AM, Fujii Masao <masao.fujii@gmail.com> wrote:

On Fri, Apr 2, 2010 at 2:58 AM, Robert Haas <robertmhaas@gmail.com> wrote:

It's probably also easy to fix so that it doesn't NEED to be documented.

The thing is, when dealing with new features, we reduce our overall
maintenance burden if we get it right the first time.  Obviously it's
too late for major changes, but minor adjustments to maintain the POLA
seem like exactly what we SHOULD be doing right now.

The attached patch implements the Heikki's proposal:

----------
ReservedBackends = superuser_reserved_connections + max_wal_senders
MaxBackends = max_connections + autovacuum_max_workers + max_wal_senders + 1
----------

This change looks like minor adjustments rather than major changes.

Instead of doing this, could we just change the logic in InitPostgres?

Current logic says we hit the connection limit if:

if (!am_superuser &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Couldn't we just change this to:

if ((!am_superuser || am_walsender) &&
ReservedBackends > 0 &&
!HaveNFreeProcs(ReservedBackends))

Seems like that'd be a whole lot simpler, if it'll do the job...

...Robert

On Tue, Apr 20, 2010 at 11:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:

Instead of doing this, could we just change the logic in InitPostgres?

Current logic says we hit the connection limit if:

       if (!am_superuser &&
               ReservedBackends > 0 &&
               !HaveNFreeProcs(ReservedBackends))

Couldn't we just change this to:

       if ((!am_superuser || am_walsender) &&
               ReservedBackends > 0 &&
               !HaveNFreeProcs(ReservedBackends))

Seems like that'd be a whole lot simpler, if it'll do the job...

It's very simple, but prevents superuser replication connection
from being established when connection limit exceeds for
non-superusers. It seems strange to me that superuser cannot use
superuser_reserved_connections slots. If we'd like to forbid
replication connection to use the slots, I think that we should
just get rid of a superuser privilege from it instead.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center