Make wal_receiver_timeout configurable per subscription

On Fri, May 16, 2025 at 9:11 PM Fujii Masao <masao.fujii@oss.nttdata.com>
wrote:

Hi,

When multiple subscribers connect to different publisher servers,
it can be useful to set different wal_receiver_timeout values for
each connection to better detect failures. However, this isn't
currently possible, which limits flexibility in managing subscriptions.

Hi,+1 for the idea.

One approach is to add wal_receiver_timeout as a parameter to
CREATE SUBSCRIPTION command, storing it in pg_subscription
so each logical replication worker can use its specific value.

Another option is to change the wal_receiver_timeout's GUC context
from PGC_SIGHUP to PGC_USERSET. This would allow setting different
values via ALTER ROLE SET command for each subscription owner -
effectively enabling per-subscription configuration. Since this
approach is simpler and likely sufficient, I'd prefer starting with this.
Thought?

Both ways LGTM,for starters we can go with changing GUC's context.

BTW, this could be extended in the future to other GUCs used by
logical replication workers, such as wal_retrieve_retry_interval.

+1 for extending this idea for other GUCs as well.

--
Thanks,
Srinath Reddy Sadipiralla
EDB: https://www.enterprisedb.com/

On Fri, May 16, 2025 at 9:11 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

When multiple subscribers connect to different publisher servers,
it can be useful to set different wal_receiver_timeout values for
each connection to better detect failures. However, this isn't
currently possible, which limits flexibility in managing subscriptions.

To address this, I'd like to propose making wal_receiver_timeout
configurable per subscription.

One approach is to add wal_receiver_timeout as a parameter to
CREATE SUBSCRIPTION command, storing it in pg_subscription
so each logical replication worker can use its specific value.

Another option is to change the wal_receiver_timeout's GUC context
from PGC_SIGHUP to PGC_USERSET. This would allow setting different
values via ALTER ROLE SET command for each subscription owner -
effectively enabling per-subscription configuration. Since this
approach is simpler and likely sufficient, I'd prefer starting with this.
Thought?

The GUC wal_receiver_interval is also used for physical replication
and logical launcher, so won't making it userset can impact those
cases as well, but maybe that is okay. However, for the specific case
you are worried about, isn't it better to make it a subscription
option as that won't have a chance to impact any other cases?

IIUC, the reason you are worried is because different publishers can
have different network latencies with subscribers, so they may want
different timing for feedback/keepalive messages.

--
With Regards,
Amit Kapila.

On Mon, May 19, 2025 at 2:48 AM Amit Kapila <amit.kapila16@gmail.com> wrote:

The GUC wal_receiver_interval is also used for physical replication
and logical launcher, so won't making it userset can impact those
cases as well, but maybe that is okay. However, for the specific case
you are worried about, isn't it better to make it a subscription
option as that won't have a chance to impact any other cases?

The advantage of Fujii-san's proposal is that it is very simple to
implement. A subscription option would indeed be better, but it would
also be considerably more complex. Why not start simple and if someone
wants to do the work to add something more complicated, that is fine?

--
Robert Haas
EDB: http://www.enterprisedb.com

On Mon, May 19, 2025 at 11:19:48AM -0400, Robert Haas wrote:

The advantage of Fujii-san's proposal is that it is very simple to
implement. A subscription option would indeed be better, but it would
also be considerably more complex. Why not start simple and if someone
wants to do the work to add something more complicated, that is fine?

Logically, adding that as an option of CREATE SUBSCRIPTION would just
be a duplication of what a connection strings are already able to do
with "options='-c foo=fooval'", isn't it?

It seems to me that the issue of downgrading wal_receiver_timeout to
become user-settable is if we're OK to allow non-superusers play with
it in the code path where it's used currently. Knowing that physical
WAL receivers are only spawned in a controlled manner by the startup
process, this does not sound like an issue.

How about logical WAL receivers, though? These are spawned by
pgoutput, but allowing wal_receiver_timeout could allow one to load
the value they want in a non-superuser context, especially in the
context of function calls (for example in the context of an index,
constraint validation, etc.). So it seems to me that the real
question is deciding if we'd be OK with that. I think that we're
actually OK here because this GUC is only used in the main apply
loops, where the GUC should be reset to its original value once we're
done applying a single logical change.
--
Michael

On Tue, 20 May 2025 at 03:16, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, May 19, 2025 at 11:19:48AM -0400, Robert Haas wrote:

The advantage of Fujii-san's proposal is that it is very simple to
implement. A subscription option would indeed be better, but it would
also be considerably more complex. Why not start simple and if someone
wants to do the work to add something more complicated, that is fine?

Logically, adding that as an option of CREATE SUBSCRIPTION would just
be a duplication of what a connection strings are already able to do
with "options='-c foo=fooval'", isn't it?

Although the value is set in the session that creates the
subscription, it will not be used by the apply worker because the
launcher process, which starts the apply worker after subscription
creation, is unaware of session-specific settings.

It seems to me that the issue of downgrading wal_receiver_timeout to
become user-settable is if we're OK to allow non-superusers play with
it in the code path where it's used currently. Knowing that physical
WAL receivers are only spawned in a controlled manner by the startup
process, this does not sound like an issue.

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Regards,
Vignesh

On Tue, May 20, 2025 at 2:13 AM vignesh C <vignesh21@gmail.com> wrote:

On Tue, 20 May 2025 at 03:16, Michael Paquier <michael@paquier.xyz> wrote:

On Mon, May 19, 2025 at 11:19:48AM -0400, Robert Haas wrote:

The advantage of Fujii-san's proposal is that it is very simple to
implement. A subscription option would indeed be better, but it would
also be considerably more complex. Why not start simple and if someone
wants to do the work to add something more complicated, that is fine?

Logically, adding that as an option of CREATE SUBSCRIPTION would just
be a duplication of what a connection strings are already able to do
with "options='-c foo=fooval'", isn't it?

I think there is a difference in the point that Vignesh made below;
the worker can detect wal_receiver_timeout change and restart.

It seems to me that the issue of downgrading wal_receiver_timeout to
become user-settable is if we're OK to allow non-superusers play with
it in the code path where it's used currently. Knowing that physical
WAL receivers are only spawned in a controlled manner by the startup
process, this does not sound like an issue.

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Right. But given changing wal_receiver_timeout doesn't happen
frequently in practice I guess this would not be a big downside of the
proposed idea.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com

On 2025/05/20 18:13, vignesh C wrote:

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Yes, this is one of the limitations of the user-settable wal_receiver_timeout
approach. If we want to change the timeout used by the apply worker without
restarting it, storing the value in pg_subscription (similar to how
synchronous_commit is handled) would be a better solution.

In that case, for example, we could set the default value of
pg_subscription.wal_receiver_timeout to -1, meaning the apply worker should
use the global wal_receiver_timeout from postgresql.conf. If the value is 0
or higher, the apply worker would use the value stored in pg_subscription.

On further thought, another downside of the user-settable approach is that
it doesn't work for parameters like wal_retrieve_retry_interval, which is
used by the logical replication launcher not the apply worker. So if we
want to support per-subscription control for non-apply workers, storing
the settings in pg_subscription might be more appropriate.

Regards,

--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

On Wed, May 21, 2025 at 6:04 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/05/20 18:13, vignesh C wrote:

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Yes, this is one of the limitations of the user-settable wal_receiver_timeout
approach. If we want to change the timeout used by the apply worker without
restarting it, storing the value in pg_subscription (similar to how
synchronous_commit is handled) would be a better solution.

In that case, for example, we could set the default value of
pg_subscription.wal_receiver_timeout to -1, meaning the apply worker should
use the global wal_receiver_timeout from postgresql.conf. If the value is 0
or higher, the apply worker would use the value stored in pg_subscription.

Yeah, I had a similar idea in my mind.

On further thought, another downside of the user-settable approach is that
it doesn't work for parameters like wal_retrieve_retry_interval, which is
used by the logical replication launcher not the apply worker. So if we
want to support per-subscription control for non-apply workers, storing
the settings in pg_subscription might be more appropriate.

Yeah, that could be an option, but one might not want to keep such
variables different for each subscription. Do you think one would like
to prefer specifying variables that only apply to the subscriber-node
in a way other than GUC? I always have this question whenever I see
GUCs like max_sync_workers_per_subscription, which are specific to
only subscriber nodes.

--
With Regards,
Amit Kapila.

On 2025/05/22 21:21, Amit Kapila wrote:

On Wed, May 21, 2025 at 6:04 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/05/20 18:13, vignesh C wrote:

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Yes, this is one of the limitations of the user-settable wal_receiver_timeout
approach. If we want to change the timeout used by the apply worker without
restarting it, storing the value in pg_subscription (similar to how
synchronous_commit is handled) would be a better solution.

In that case, for example, we could set the default value of
pg_subscription.wal_receiver_timeout to -1, meaning the apply worker should
use the global wal_receiver_timeout from postgresql.conf. If the value is 0
or higher, the apply worker would use the value stored in pg_subscription.

Yeah, I had a similar idea in my mind.

OK, I've implemented two patches:

- 0001 makes the wal_receiver_timeout GUC user-settable.
- 0002 adds support for setting wal_receiver_timeout per subscription.
It depends on the changes in 0001.

With both patches applied, wal_receiver_timeout can be set per role or
per database using ALTER ROLE or ALTER DATABASE (from 0001), and also
per subscription using CREATE SUBSCRIPTION or ALTER SUBSCRIPTION (from 0002).
The per-subscription value is stored in pg_subscription.subwalrcvtimeout,
and it overrides the global setting of wal_receiver_timeout for that
subscription's apply worker. The default is -1, meaning the global setting
(from server config, command line, role, or database) is used.

I'll add this to the next CommitFest.

On further thought, another downside of the user-settable approach is that
it doesn't work for parameters like wal_retrieve_retry_interval, which is
used by the logical replication launcher not the apply worker. So if we
want to support per-subscription control for non-apply workers, storing
the settings in pg_subscription might be more appropriate.

Yeah, that could be an option, but one might not want to keep such
variables different for each subscription. Do you think one would like
to prefer specifying variables that only apply to the subscriber-node
in a way other than GUC? I always have this question whenever I see
GUCs like max_sync_workers_per_subscription, which are specific to
only subscriber nodes.

I like still using the xxx_per_subscription GUC as the default,
and also allowing it to be overridden per subscription. This seems
intuitive for some users and adds useful flexibility.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On 2025/05/28 0:36, Fujii Masao wrote:

On 2025/05/22 21:21, Amit Kapila wrote:

On Wed, May 21, 2025 at 6:04 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:

On 2025/05/20 18:13, vignesh C wrote:

If we set the wal_receiver_timeout configuration using ALTER ROLE for
the subscription owner's role, the apply worker will start with that
value. However, any changes made via ALTER ROLE ... SET
wal_receiver_timeout will not take effect for an already running apply
worker unless the subscription is disabled and re-enabled. In
contrast, this is handled automatically during CREATE SUBSCRIPTION,
where parameter changes are detected.

Yes, this is one of the limitations of the user-settable wal_receiver_timeout
approach. If we want to change the timeout used by the apply worker without
restarting it, storing the value in pg_subscription (similar to how
synchronous_commit is handled) would be a better solution.

In that case, for example, we could set the default value of
pg_subscription.wal_receiver_timeout to -1, meaning the apply worker should
use the global wal_receiver_timeout from postgresql.conf. If the value is 0
or higher, the apply worker would use the value stored in pg_subscription.

Yeah, I had a similar idea in my mind.

OK, I've implemented two patches:

  - 0001 makes the wal_receiver_timeout GUC user-settable.
  - 0002 adds support for setting wal_receiver_timeout per subscription.
    It depends on the changes in 0001.

With both patches applied, wal_receiver_timeout can be set per role or
per database using ALTER ROLE or ALTER DATABASE (from 0001), and also
per subscription using CREATE SUBSCRIPTION or ALTER SUBSCRIPTION (from 0002).
The per-subscription value is stored in pg_subscription.subwalrcvtimeout,
and it overrides the global setting of wal_receiver_timeout for that
subscription's apply worker. The default is -1, meaning the global setting
(from server config, command line, role, or database) is used.

I've attached the rebased patches.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation

On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:

I've attached the rebased patches.

Attached are the rebased versions of the patches.

Regards,

--
Fujii Masao

On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
<masao.fujii@oss.nttdata.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

References: <a1414b64-bf58-43a6-8494-9704975a41e9@oss.nttdata.com>
<CAA4eK1JtSN2OW4+xPMOoVfYF5LG+ZdBQ8LMAk1h_mCd5SsuCxw@mail.gmail.com>
<CA+TgmobgPuxLWMbTzBE72yKDQJTXpCnGjtCN3v5N=u_F3uD_nw@mail.gmail.com>
<aCumuj3V5geOw8YV@paquier.xyz>
<CALDaNm0Ro-Z0JdsuZxEYRQxqdOOY2U3vRrPtRU=re4CB8Ee-2A@mail.gmail.com>
<3ed7e711-102f-496d-93b8-8b2619d4d875@oss.nttdata.com>
<CAA4eK1LSVODWq5aC92Q2PuHRiGqs68bZmumYbC-D7d39MCvukw@mail.gmail.com>
<5780e93c-7183-4aeb-b3a9-0a5ba0ff7e02@oss.nttdata.com>
<adf8214d-f2ae-4777-9ba0-33f18ab77e0b@oss.nttdata.com>
<CAHGQGwG82P4s6tmYK=aEm-T7QfGJBZvXo=WZfckMkffsX6DZjQ@mail.gmail.com>
<CAHGQGwHq0hP8zZVxaRrvoqD6ZJsWsTO8E_4QqPn5X3bEfEZSMQ@mail.gmail.com>
User-Agent: mu4e 1.12.12; emacs 29.3
Hi, Fujii

Date: Thu, 05 Feb 2026 12:06:45 +0800

On Thu, 05 Feb 2026 at 09:33, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
<masao.fujii@oss.nttdata.com> wrote:

I've attached the rebased patches.

Attached are the rebased versions of the patches.

I've rebased the patches again.

Thanks for updating the patches.
I have one small comment on v4-0002:

@@ -104,6 +105,7 @@ typedef struct SubOpts
int32 maxretention;
char *origin;
XLogRecPtr lsn;
+ char *wal_receiver_timeout;
} SubOpts;

According to the comment above the SubOpts struct:

Structure to hold a bitmap representing the user-provided CREATE/ALTER
SUBSCRIPTION command options and the parsed/default values of each of them.

Since `wal_receiver_timeout` is a GUC-style interval value (typically stored as
integer milliseconds), wouldn't it be better to use an int32 here instead of a
string?

Regards,

--
Fujii Masao

[2. text/x-diff; v4-0001-Make-GUC-wal_receiver_timeout-user-settable.patch]...

[3. text/x-diff; v4-0002-Add-per-subscription-wal_receiver_timeout-setting.patch]...

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

On Feb 5, 2026, at 08:33, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
<masao.fujii@oss.nttdata.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/

On Thu, Feb 5, 2026 at 1:06 PM Japin Li <japinli@hotmail.com> wrote:

Thanks for updating the patches.
I have one small comment on v4-0002:

Thanks for the review!

@@ -104,6 +105,7 @@ typedef struct SubOpts
int32           maxretention;
char       *origin;
XLogRecPtr      lsn;
+       char       *wal_receiver_timeout;
} SubOpts;

According to the comment above the SubOpts struct:

Structure to hold a bitmap representing the user-provided CREATE/ALTER
SUBSCRIPTION command options and the parsed/default values of each of them.

Since `wal_receiver_timeout` is a GUC-style interval value (typically stored as
integer milliseconds), wouldn't it be better to use an int32 here instead of a
string?

The wal_receiver_timeout value in CREATE SUBSCRIPTION can include a unit
(for example, 10s), not just a plain integer. Because of that, we can't store it
in an int32, I think.

Regards,

--
Fujii Masao

On Thu, 05 Feb 2026 at 23:40, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Feb 5, 2026 at 1:06 PM Japin Li <japinli@hotmail.com> wrote:

Thanks for updating the patches.
I have one small comment on v4-0002:

Thanks for the review!

@@ -104,6 +105,7 @@ typedef struct SubOpts
int32           maxretention;
char       *origin;
XLogRecPtr      lsn;
+       char       *wal_receiver_timeout;
} SubOpts;

According to the comment above the SubOpts struct:

Structure to hold a bitmap representing the user-provided CREATE/ALTER
SUBSCRIPTION command options and the parsed/default values of each of them.

Since `wal_receiver_timeout` is a GUC-style interval value (typically stored as
integer milliseconds), wouldn't it be better to use an int32 here instead of a
string?

The wal_receiver_timeout value in CREATE SUBSCRIPTION can include a unit
(for example, 10s), not just a plain integer. Because of that, we can't store it
in an int32, I think.

If we stored it as an integer, an input such as '1min' would be normalized to
60000 (milliseconds) and lose its unit.

That would make it inconsistent with the original user input shown in pg_subscription.
So we keep it as a string, right?

Regards,

--
Fujii Masao

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

On Fri, Feb 6, 2026 at 10:50 AM Japin Li <japinli@hotmail.com> wrote:

If we stored it as an integer, an input such as '1min' would be normalized to
60000 (milliseconds) and lose its unit.

That would make it inconsistent with the original user input shown in pg_subscription.
So we keep it as a string, right?

Yes, I think.

Regards,

--
Fujii Masao

On Thu, 05 Feb 2026 at 16:04, Chao Li <li.evan.chao@gmail.com> wrote:

On Feb 5, 2026, at 08:33, Fujii Masao <masao.fujii@gmail.com> wrote:

On Thu, Oct 23, 2025 at 5:19 PM Fujii Masao <masao.fujii@gmail.com> wrote:

On Mon, Jul 14, 2025 at 10:27 PM Fujii Masao
<masao.fujii@oss.nttdata.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.

On Fri, Feb 6, 2026 at 2:03 PM Chao Li <li.evan.chao@gmail.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