Re: Connection slots reserved for replication

From: Oleksii Kliukin <alexk(at)hintbits(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: petr(dot)jelinek(at)2ndquadrant(dot)com, cyberdemn(at)gmail(dot)com, sfrost(at)snowman(dot)net, magnus(at)hagander(dot)net, sawada(dot)mshk(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Connection slots reserved for replication
Date: 2019-01-25 17:37:34
Message-ID: 72696420-9216-47A9-9515-FA3177A80C17@hintbits.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

Hello,

> On 24. Jan 2019, at 13:47, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:

Thank you for the review.I took a liberty to address it with v9.

>
> Documentation looks fine for me. By the way, a comment for the
> caller-site of CheckRequreParameterValues() in xlog.c looks
> somewhat stale.
>
>> /* Check to see if any changes to max_connections give problems */
>> CheckRequiredParameterValues();
>
> may be better something like this?:
>
>> /* Check to see if any parameter change gives a problem on replication */

I changed it to "Check if any parameter change gives a problem on recovery”, as I think it is independent of the [streaming] replication, but I still don’t like the wording, as it just duplicate the comment at the definition of CheckRequiredParameterValues. Maybe remove the comment altogether?

>
>
> In postinit.c:
>> /*
>> * The last few connection slots are reserved for superusers.
>> */
>> if ((!am_superuser && !am_walsender) &&
>> ReservedBackends > 0 &&
>
> This is forgetting about explaing about walsenders.
>
>> The last few connection slots are reserved for
>> superusers. Replication connections don't share the same slot
>> pool.
>
> Or something?

I changed it to

+ * The last few connection slots are reserved for superusers.
+ * Replication connections are drawn from a separate pool and
+ * not limited by max_connections or superuser_reserved_connections.

>
> And the parentheses enclosing "!am_superuser..walsender" seems no
> longer needed.
>
>
> In guc.c:
> - /* see max_connections and max_wal_senders */
> + /* see max_connections */
>
> I don't understand for what reason we should see max_connections
> just above. (Or even max_wal_senders) Do we need this comment?
> (There're some other instances, but they wont'be for this patch.)

I don’t understand what is it pointing to as well, so I’ve removed it.

>
>
> In pg_controldata.c:
> + printf(_("max_wal_senders setting: %d\n"),
> + ControlFile->max_wal_senders);
> printf(_("max_worker_processes setting: %d\n"),
> ControlFile->max_worker_processes);
> printf(_("max_prepared_xacts setting: %d\n"),
>
> The added item seems to want some additional spaces.

Good catch, fixed.

Attached is v9. I also bumped up the PG_CONTROL_VERSION to 1200 per the prior comment by Robert.

Cheers,
Oleksii

Attachment Content-Type Size
replication_reserved_connections_v9.patch application/octet-stream 19.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jim Finnerty 2019-01-25 17:41:52 Re: Use zero for nullness estimates of system attributes
Previous Message Tom Lane 2019-01-25 17:16:49 Re: pg_upgrade: Pass -j down to vacuumdb