Thread
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Add per-subscription wal_receiver_timeout setting.
- fb80f388f4a1 19 (unreleased) landed
-
Make GUC wal_receiver_timeout user-settable.
- 8a6af3ad0879 19 (unreleased) landed
-
Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-16T15:40:53Z
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. 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? BTW, this could be extended in the future to other GUCs used by logical replication workers, such as wal_retrieve_retry_interval. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
-
Re: Make wal_receiver_timeout configurable per subscription
Srinath Reddy Sadipiralla <srinath2133@gmail.com> — 2025-05-16T16:47:28Z
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/
-
Re: Make wal_receiver_timeout configurable per subscription
Amit Kapila <amit.kapila16@gmail.com> — 2025-05-19T06:48:04Z
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.
-
Re: Make wal_receiver_timeout configurable per subscription
Robert Haas <robertmhaas@gmail.com> — 2025-05-19T15:19:48Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Michael Paquier <michael@paquier.xyz> — 2025-05-19T21:46:34Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
vignesh C <vignesh21@gmail.com> — 2025-05-20T09:13:27Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Masahiko Sawada <sawada.mshk@gmail.com> — 2025-05-21T00:25:05Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-21T12:34:20Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Amit Kapila <amit.kapila16@gmail.com> — 2025-05-22T12:21:23Z
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.
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-05-27T15:36:01Z
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 -
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@oss.nttdata.com> — 2025-07-14T13:27:13Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2025-10-23T08:19:49Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-05T00:33:16Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Japin Li <japinli@hotmail.com> — 2026-02-05T04:06:45Z
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. -
Re: Make wal_receiver_timeout configurable per subscription
Chao Li <li.evan.chao@gmail.com> — 2026-02-05T08:04:12Z
> 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/ -
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-05T14:40:49Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Japin Li <japinli@hotmail.com> — 2026-02-06T01:50:27Z
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.
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-06T04:32:36Z
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
-
Re: Make wal_receiver_timeout configurable per subscription
Japin Li <japinli@hotmail.com> — 2026-02-06T06:44:35Z
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. -
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-13T15:51:37Z
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 -
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-13T15:52:56Z
On Fri, Feb 6, 2026 at 3:44 PM Japin Li <japinli@hotmail.com> wrote: > 1. > Typo, s/timeeout/timeout/g. Fixed. Thanks for the review! > 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? With this approach, a command like CREATE SUBSCRIPTION using a wal_receiver_timeout value such as '-1 ' (i.e., -1 followed by whitespace) would fail, since it would not be interpreted as -1. I don't think that's desirable behavior. So it would be better to use parse_int() so that such input is handled correctly. Regards, -- Fujii Masao
-
Re: Make wal_receiver_timeout configurable per subscription
Chao Li <li.evan.chao@gmail.com> — 2026-02-13T23:37:07Z
> On Feb 13, 2026, at 23:51, Fujii Masao <masao.fujii@gmail.com> wrote: > > 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 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/ -
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-16T07:48:49Z
On Sat, Feb 14, 2026 at 8:37 AM Chao Li <li.evan.chao@gmail.com> wrote: > 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. IMO the current comment is sufficient, so I left it unchanged. + /* 180000 should be changed to 190000 */ + if (pset.sversion >= 180000) + appendPQExpBuffer(&buf, Since I started the patch before v19dev was created, I temporarily used 180000 and planned to update it to 190000 later, but forgot to do so. This is now fixed. + /* + * Log the current wal_receiver_timeout GUC value (in milliseconds) as a + * debug message to verify it was set correctly. + */ + elog(DEBUG1, "logical replication worker for subscription \"%s\" wal_receiver_timeout: %d ms", + MySubscription->name, wal_receiver_timeout); Previously, this debug message was emitted every time set_wal_receiver_timeout() was called, even when the value had not changed. For example, ALTER SUBSCRIPTION ... SET (synchronous_commit = true) would trigger the message, which seemed strange. I updated the code so the message is logged only when the effective wal_receiver_timeout value used by the worker actually changes. Attached are the updated patches. Regards, -- Fujii Masao
-
Re: Make wal_receiver_timeout configurable per subscription
Fujii Masao <masao.fujii@gmail.com> — 2026-02-19T16:03:58Z
On Mon, Feb 16, 2026 at 4:48 PM Fujii Masao <masao.fujii@gmail.com> wrote: > Attached are the updated patches. I've pushed the patches. Thanks! Regards, -- Fujii Masao