| 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-13 23:37:07 |
| Message-ID: | 41DCF874-98D8-4A3D-A434-3A06FE2A6681@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Feb 13, 2026, at 23:51, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>
> On Fri, Feb 6, 2026 at 2:03 PM Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>> 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.
>
> Thanks for the review!
>
>
>> 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 we use atoi(), a command like CREATE SUBSCRIPTION with an invalid
> wal_receiver_timeout value such as '-1invalid' would succeed, since atoi()
> interprets it as -1. I don't think that's desirable behavior. So it would be
> better to use parse_int() so that such invalid input is properly rejected.
>
> Regards,
>
> --
> Fujii Masao
I realized atoi(“-1invalid”) would return -1, but I thought that would be an imagined use case. I’m fine if you insist to use parse_int. Maybe we can enhance the comment. set_config_option does the test and parse_int is used to skip -1.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Cary Huang | 2026-02-14 00:21:12 | Re: Add pg_get_publication_ddl function |
| Previous Message | Nathan Bossart | 2026-02-13 23:09:25 | Re: Speed up COPY FROM text/CSV parsing using SIMD |