Re: Connection slots reserved for replication

Hello,

> On 24. Jan 2019, at 13:47, Kyotaro HORIGUCHI <horiguchi.kyotaro@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