> On Feb 5, 2026, at 08:33, Fujii Masao <[email protected]> wrote:
> 
> On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <[email protected]> wrote:
>> 
>> On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
>> <[email protected]> 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/






Reply via email to