Re: Connection slots reserved for replication

From: Oleksii Kliukin <alexk(at)hintbits(dot)com>
To: Alexander Kukushkin <cyberdemn(at)gmail(dot)com>, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, magnus(at)hagander(dot)net, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, "PostgreSQL-development" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Connection slots reserved for replication
Date: 2019-01-02 09:21:56
Message-ID: 1546420916.2223110.1623350128.4937B8F5@webmail.messagingengine.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 17, 2018, at 14:10, Alexander Kukushkin wrote:
> Hi,
>
> On Thu, 6 Dec 2018 at 00:55, Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com> wrote:
> > > Does excluding WAL senders from the max_connections limit and including max_wal_senders in MaxBackends also imply that we need to add max_wal_senders to the list at xlog.c: CheckRequiredParameterValues, requiring its value on the replica to be not lower than the one on the primary?
> > >
> >
> > I think it does, we need the proc slots for walsenders on the standby
> > same way we do for normal backends.
>
> You are absolutely right. Attaching the new version of the patch.

Thank you. I've checked that the replica correctly complains when its value of max_wal_senders is lower than the one on the primary at v6.

As stated in my previous comment, I think we should retain the specific error message on exceeding max_wal_senders, instead of showing the generic "too many clients already'. Attached is the patch that fixes this small thing. I've also rebased it against the master and took a liberty of naming it v7. It makes me wondering why don't we apply the same level of details to the regular out of connection message and don't show the actual value of max_connections in the error text?

The code diff to the previous version is rather small:

diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
index d1a8113cb6..df073673f5 100644
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -2273,8 +2273,8 @@ InitWalSenderSlot(void)
Assert(MyWalSnd == NULL);

/*
- * Find a free walsender slot and reserve it. If this fails, we must be
- * out of WalSnd structures.
+ * Find a free walsender slot and reserve it. This must not fail due
+ * to the prior check for free walsenders at InitProcess.
*/
for (i = 0; i < max_wal_senders; i++)
{
@@ -2310,13 +2310,7 @@ InitWalSenderSlot(void)
break;
}
}
- if (MyWalSnd == NULL)
- ereport(FATAL,
- (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
- errmsg("number of requested standby connections "
- "exceeds max_wal_senders (currently %d)",
- max_wal_senders)));
-
+ Assert(MyWalSnd != NULL);
/* Arrange to clean up at walsender exit */
on_shmem_exit(WalSndKill, 0);
}

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 33387fb71b..fe33fc42e3 100644
@ -341,6 +353,12 @@ InitProcess(void)
* in the autovacuum case?
*/
SpinLockRelease(ProcStructLock);
+ if (am_walsender)
+ ereport(FATAL,
+ (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
+ errmsg("number of requested standby connections "
+ "exceeds max_wal_senders (currently %d)",
+ max_wal_senders)));
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("sorry, too many clients already")));

Cheers,
Oleksii

Attachment Content-Type Size
replication_reserved_connections_v7.patch text/plain 16.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nikolay Shaplov 2019-01-02 09:58:15 Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead
Previous Message Andreas Karlsson 2019-01-02 04:31:30 Re: Early WIP/PoC for inlining CTEs