Re: Make wal_receiver_timeout configurable per subscription

From: Japin Li <japinli(at)hotmail(dot)com>
To: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Make wal_receiver_timeout configurable per subscription
Date: 2026-02-06 06:44:35
Message-ID: MEAPR01MB3031A8497426D60CF7E3BB2FB666A@MEAPR01MB3031.ausprd01.prod.outlook.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 05 Feb 2026 at 16:04, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>> On Feb 5, 2026, at 08:33, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>
>> On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>
>>> On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
>>> <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>>> I've attached the rebased patches.
>>>
>>> Attached are the rebased versions of the patches.
>>
>> I've rebased the patches again.
>>
>> Regards,
>>
>> --
>> Fujii Masao
>> <v4-0001-Make-GUC-wal_receiver_timeout-user-settable.patch><v4-0002-Add-per-subscription-wal_receiver_timeout-setting.patch>
>
> Hi Fujii-san,
>
> I applied the patch locally and played with it a bit. In short, it adds a new subscription option that allows overriding the GUC wal_receiver_timeout for a subscription’s apply worker. The changes look solid overall, and the new option worked as expected in my manual testing.
>
> I have only one small comment:
> ```
> + /*
> + * Test if the given value is valid for wal_receiver_timeeout GUC.
> + * Skip this test if the value is -1, since -1 is allowed for the
> + * wal_receiver_timeout subscription option, but not for the GUC
> + * itself.
> + */
> + parsed = parse_int(opts->wal_receiver_timeout, &val, 0, NULL);
> + if (!parsed || val != -1)
> + (void) set_config_option("wal_receiver_timeout", opts->wal_receiver_timeout,
> + PGC_BACKEND, PGC_S_TEST, GUC_ACTION_SET,
> + false, 0, false);
> ```
>

1.
Typo, s/timeeout/timeout/g.

2.
The comment mentions skipping only "-1".
Since we already use strcmp(... , "-1") later in the code, wouldn't it be
better to use the same check here too?

+ if (strcmp(subinfo->subwalrcvtimeout, "-1") != 0)
+ appendPQExpBuffer(query, ", wal_receiver_timeout = %s", fmtId(subinfo->subwalrcvtimeout));
+

> Here, parse_int() is also from GUC, with flag 0, it will reject any value with units such as “1s” or “7d”. So in practice, the only purpose of calling parse_int() here is to detect the special value “-1”.
>
> Given that, I think using atoi() directly may be simpler and easier to read. For example:
> ```
> if (atoi(opts->wal_receiver_timeout) != -1)
> /* if value is not -1, then test if the given value is valid for wal_receiver_timeeout GUC.
> (void) set_config_option("wal_receiver_timeout", opts->wal_receiver_timeout,
> PGC_BACKEND, PGC_S_TEST, GUC_ACTION_SET,
> false, 0, false);
> ```
>
> I tried this locally and `make check` still passed.
>
> Similarly, later in set_wal_receiver_timeout(), MySubscription->walrcvtimeout has already been validated, so we could also use atoi() there instead of parse_int().
>
> Best regards,
> --
> Chao Li (Evan)
> HighGo Software Co., Ltd.
> https://www.highgo.com/

--
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrey Borodin 2026-02-06 07:02:35 Windows locales and tests portability
Previous Message Michael Paquier 2026-02-06 06:44:27 Re: Small fixes for incorrect error messages