Re: GUC-ify walsender MAX_SEND_SIZE constant

Thank you very much for your review.

I generally agree with your suggestions, so just applied them.
You can find the new patch in the attached file.

Best
Majid

On Tue, 6 Feb 2024 at 09:26, Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> On Fri, Jan 19, 2024 at 11:04:50PM +0330, Majid Garoosi wrote:
> > However, this value does not need to be larger than wal_segment_size,
> > thus its checker function returns false if a larger value is set for
> > this.
> >
> > This is my first patch. So, I hope I haven't done something wrong. :'D
>
> You've done nothing wrong. Thanks for the patch!
>
> + if (*newval > wal_segment_size)
> + return false;
> + return true;
>
> I was not sure first that we need a dynamic check, but I could get why
> somebody may want to make it higher than 1MB these days.
>
> The patch is missing a couple of things:
> - Documentation in doc/src/sgml/config.sgml, that has a section for
> "Sending Servers".
> - It is not listed in postgresql.conf.sample. I would suggest to put
> it in REPLICATION -> Sending Servers.
> The default value of 128kB should be mentioned in both cases.
>
> - * We don't have a good idea of what a good value would be; there's some
> - * overhead per message in both walsender and walreceiver, but on the
> other
> - * hand sending large batches makes walsender less responsive to signals
> - * because signals are checked only between messages. 128kB (with
> - * default 8k blocks) seems like a reasonable guess for now.
> [...]
> + gettext_noop("Walsender procedure consists of a loop, reading
> wal_sender_max_send_size "
> + "bytes of WALs from disk and sending them to the receiver.
> Sending large "
> + "batches makes walsender less responsive to signals."),
>
> This is removing some information about why it may be a bad idea to
> use a too low value (message overhead) and why it may be a bad idea to
> use a too large value (responsiveness). I would suggest to remove the
> second gettext_noop() in guc_tables.c and move all this information to
> config.sgml with the description of the new GUC. Perhaps just put it
> after wal_sender_timeout in the sample file and the docs?
>
> Three comments in walsender.c still mention MAX_SEND_SIZE. These
> should be switched to mention the GUC instead.
>
> I am switching the patch as waiting on author for now.
> --
> Michael
>