| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
| Cc: | 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-05 08:04:12 |
| Message-ID: | A82E6A54-036A-4DE8-AD97-5C071CF16CA3@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> 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);
```
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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zhijie Hou (Fujitsu) | 2026-02-05 08:12:33 | RE: Warn when creating or enabling a subscription with max_logical_replication_workers = 0 |
| Previous Message | John Naylor | 2026-02-05 07:48:44 | Re: refactor architecture-specific popcount code |