Hello, > On 24. Jan 2019, at 13:47, Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.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
replication_reserved_connections_v9.patch
Description: Binary data
