Re: Warn when creating or enabling a subscription with max_logical_replication_workers = 0

Hi Yugo,

Thank you for working on this. I think the warning will save people
real debugging time.

Shubhra already covered the basic behaviour on 2026-10-06 (thanks,
Shubhra), and I see the same results on current master. To avoid
repeating her review, below I focus on what I think is new: the patch
no longer applies, ALTER SUBSCRIPTION ... SET (retain_dead_tuples)
gives no warning, the max_worker_processes = 0 case, and a few small
message-style details.

I reviewed 0001-Warn-when-creating-or-enabling-a-subscription-with-l.patch
(the only version of your patch in the thread, posted 2026-02-04; I
did not test the pg_stat_subscription_stats patch from Rafia).

Tested against master c074737f687f604b64f6ee830fccbbe854719823
(2026-11-09), Debian bookworm container, ./configure --enable-cassert
--enable-debug --enable-tap-tests.

Applying
- "git apply" fails on current master (hunks at subscriptioncmds.c:906
and :1694 no longer match), which agrees with the cfbot "Needs rebase"
status. "patch -p1 -F3" applies it with offsets of +196 and +461 lines
and fuzz 2 and 1, so the rebase looks mechanical. Please post a
rebased version.

Build and tests (with the patch applied)
- Build is clean, no compiler warnings.
- make check: all 243 tests passed.
- make -C src/test/subscription check: 39 files, 616 tests, PASS.
- None of these exercise the new code, because the regression and TAP
clusters run with the default max_logical_replication_workers = 4.

Manual testing
Setup: publisher on port 5433 (wal_level = logical, table t,
publication pub1), subscriber on port 5434, default CREATE
SUBSCRIPTION options unless noted.

With max_logical_replication_workers = 0 on the subscriber (no
"logical replication launcher" process present):
- CREATE SUBSCRIPTION (enabled, connected):
NOTICE: created replication slot "s_a" on publisher
WARNING: subscription was created, but logical replication is disabled
HINT: To initiate replication, set
"max_logical_replication_workers" to a non zero value.
- ALTER SUBSCRIPTION ... ENABLE (also for a subscription created with
connect = false):
WARNING: subscription was enabled, but logical replication is disabled
HINT: (same hint)
- ALTER SUBSCRIPTION ... DISABLE, and CREATE SUBSCRIPTION ... WITH
(enabled = false): no new warning, as expected.
- CREATE SUBSCRIPTION ... WITH (connect = false): only the existing
"subscription was created, but is not connected" warning, as expected.
(connect = false together with enabled = true is rejected, so that
combination cannot reach the new code.)
- CREATE SUBSCRIPTION ... WITH (enabled = false, retain_dead_tuples =
true): the new "subscription was created, but logical replication is
disabled" warning is shown, right after the existing "deleted rows to
detect conflicts would not be removed until the subscription is
enabled" warning. See comment 2.
- ALTER SUBSCRIPTION ... SET (retain_dead_tuples = true) on a disabled
subscription, OWNER TO, and REFRESH PUBLICATION: no new warning. See
comment 3.

With max_logical_replication_workers = 4: none of the above commands
emit the new warning.

With max_logical_replication_workers = 4 but max_worker_processes = 0,
the launcher is not running (no launcher process in pg_stat_activity),
and CREATE SUBSCRIPTION and ENABLE succeed silently. With
max_worker_processes = 1 the launcher runs, and there is no warning
either. So the patch only covers the GUC = 0 case, not the "no worker
process available" cases. I think that is acceptable given the
discussion upthread, but it could be mentioned in the commit message
or in a comment.

Comments

1. Design. Peter Smith and Hou-san questioned checking only the GUC,
and preferred a warning based on available workers. You replied
(2026-04-13) that there is no cheap way to do that check from a
backend, and treated it as a separate matter. I agree with keeping
this patch limited to the GUC = 0 case, since that is the case where
nothing will ever start and the launcher does not even exist. Rafia
raised the case of the GUC being changed to 0 and the server restarted
with no CREATE/ALTER command issued afterwards; that is outside what a
warning in these commands can do, so I think it needs a separate
mechanism, as she proposed. It would help to state the scope
explicitly, in the commit message and in a short comment above the
checks, to close that discussion.

2. Hint wording for the retain_dead_tuples case (also raised by
Shubhra Jain). When the subscription is created with enabled = false
and retain_dead_tuples = true, the user is told "To initiate
replication, set ..." although the subscription is not enabled and no
replication would start in any case. The launcher is needed there only
to create the conflict detection slot. Either use a separate message
for that branch (for example, one that says the conflict detection
slot cannot be created because logical replication is disabled), or
restrict the current hint to the opts.enabled case.

3. ALTER SUBSCRIPTION ... SET (retain_dead_tuples = ...) also calls
ApplyLauncherWakeupAtCommit() (the "manage the replication slot for
conflict detection" path), and there is no warning with the GUC at 0,
even though CREATE gives one for the same option. This looks
inconsistent. Either cover it as well or document the reason for
leaving it out. The call in the owner-change path is not affected
unless the subscription is enabled, so leaving it out there looks
right.

4. Message wording.
- Hint: "non zero" should be "non-zero" (create_subscription.sgml and
other messages use "non-zero").
- The errmsg is lower case with no trailing period, and the errhint
is a full sentence ending in a period. This conforms to the message
style guide.
- The GUC name is hard-coded in the hint string. Nearby code passes
GUC names as %s arguments (for example, "Consider setting \"%s\" to
true.") so that translated strings can be shared. Since the same hint
is used twice, errhint("To initiate replication, set \"%s\" to a
non-zero value.", "max_logical_replication_workers") would let both
copies share one translatable string.
- "logical replication is disabled" is slightly ambiguous, since
"disabled" is also what a subscription is when subenabled is false.
Something like "logical replication workers cannot be started because
\"max_logical_replication_workers\" is 0" might be clearer, but I do
not have a strong opinion.
- The new ereport() calls use the extra-parenthesis form, which
matches the neighbouring "is not connected" warning, so that is fine.
Much of the newer code in this file omits the parentheses, so either
is acceptable.

5. Code structure. The same check is duplicated in
CreateSubscription() and AlterSubscription(). A small static helper
that takes the first part of the message (created/enabled) would avoid
duplicating the hint, but with only two callers this is a matter of
taste.

6. Tests. The patch has no test. As far as I can see, the only places
in src/test that set max_logical_replication_workers are
010_truncate.pl and 031_column_list.pl (and
authentication/t/007_pre_auth.pl), and none checks for this. A small
TAP test in src/test/subscription that starts a subscriber with
max_logical_replication_workers = 0 and checks, using psql's stderr,
for the warning on CREATE SUBSCRIPTION and ENABLE, and for its absence
on DISABLE, enabled = false, and with the default setting, would cover
the new code. Shubhra Jain has offered to write one, so please let her
know whether you want that. The core regression tests cannot cover
this because the setting needs a restart.

7. Documentation. No documentation change is included. I do not think
one is required, but a sentence in the max_logical_replication_workers
entry in config.sgml saying that CREATE/ALTER SUBSCRIPTION warn when
it is zero would make the behaviour discoverable. Optional.

Verdict: Waiting on Author.
Reasons: (a) the patch needs a rebase (git apply fails on current
master); (b) the hint is misleading for the enabled = false +
retain_dead_tuples case; (c) the ALTER SUBSCRIPTION SET
(retain_dead_tuples) path is inconsistent with CREATE; (d) there is no
test; (e) minor wording fixes ("non-zero", %s for the GUC name). The
core approach looks correct to me and the main scenarios behave as
intended, so I expect this to be close to Ready for Committer after
the next version.

Thanks again, and I am happy to test the next version.

Regards,
Joao Rito

Em seg., 9 de nov. de 2026 às 22:37, Rafia Sabih
<rafia(dot)pghackers(at)gmail(dot)com> escreveu:
>
>
>
> On Mon, 13 Apr 2026 at 09:04, Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
>>
>> On Fri, 6 Feb 2026 18:28:06 +1100
>> Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>>
>> > On Fri, Feb 6, 2026 at 3:37 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
>> > >
>> > > On Fri, 6 Feb 2026 09:58:02 +1100
>> > > Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>> > >
>> > > > On Thu, Feb 5, 2026 at 7:12 PM Zhijie Hou (Fujitsu)
>> > > > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>> > > > >
>> > > > > On Thursday, February 5, 2026 3:47 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>> > > > > > On Thu, Feb 5, 2026 at 12:12 PM Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:
>> > > > > > >
>> > > > > > > On Wed, 4 Feb 2026 17:26:25 +1100
>> > > > > > > Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>> > > > > > >
>> > > > ...
>> > > > > >
>> > > > > > Oh right, I mistook that you had run out of logical replication "workers", but in
>> > > > > > fact, because max_logical_replication_workers = 0 the main "logical
>> > > > > > replication launcher" process had failed to start, so logical replication was
>> > > > > > entirely disabled.
>> > > > > >
>> > > > > > See code: in backend/replication/logical/launcher.c
>> > > > > >
>> > > > > > ApplyLauncherRegister(void)
>> > > > > > {
>> > > > > > ...
>> > > > > > if (max_logical_replication_workers == 0 || IsBinaryUpgrade)
>> > > > > > return;
>> > > > > >
>> > > > > > ~~~
>> > > > > >
>> > > > > > Given this, I felt that instead of testing the GUC, what you really want to know
>> > > > > > is just whether that "logical replication launcher" is running or not.
>> > > > > >
>> > > > > > And that launcher pid is already tested when the Subscription commands send
>> > > > > > a "kill" to the launcher. e.g. see function ApplyLauncherWakeup.
>> > > > > >
>> > > > > > So, here is a diff patch, of what I tried:
>> > > > > >
>> > > > > > ------
>> > > > > > diff --git a/src/backend/replication/logical/launcher.c
>> > > > > > b/src/backend/replication/logical/launcher.c
>> > > > > > index 3ed86480be2..f880380ce4e 100644
>> > > > > > --- a/src/backend/replication/logical/launcher.c
>> > > > > > +++ b/src/backend/replication/logical/launcher.c
>> > > > > > @@ -1195,6 +1195,13 @@ ApplyLauncherWakeup(void) {
>> > > > > > if (LogicalRepCtx->launcher_pid != 0)
>> > > > > > kill(LogicalRepCtx->launcher_pid, SIGUSR1);
>> > > > > > + else
>> > > > > > + {
>> > > > > > + if (max_logical_replication_workers == 0)
>> > > > > > + ereport(WARNING,
>> > > > > > + errmsg("Logical replication is
>> > > > > > currently disabled"),
>> > > > > > + errhint("\"%s\" is 0.",
>> > > > > > "max_logical_replication_workers"));
>> > > > > > + }
>> > > > > > }
>> > > > > > ------
>> > > > > >
>> > > > > > Thoughts?
>> > > > >
>> > > > > I think this is not the right place to check this issue. The launcher might fail
>> > > > > for some reasons and restart soon (pid will be set to 0), in which case this
>> > > > > warning wouldn't be appropriate.
>> > > >
>> > > > AFAIK, that's not possible. My warning is guarded by checking
>> > > > max_logical_replication_workers == 0. And in that case, the launcher
>> > > > cannot "fail" because it was never registered/started in the first
>> > > > place.
>> > >
>> > > I also initially considered emitting the warning in
>> > > ApplyLauncherWakeup() after checking　max_logical_replication_workers == 0.
>> > > However, I think checking pid == 0 is sufficient.
>> > >
>> > > Even when max_logical_replication_workers is non-zero and the launcher is
>> > > normally running, it could still be killed by user action or by the
>> > > OS, although such cases should be rare. In that situation, emitting the
>> > > same warning would not be appropriate.
>> >
>> > Sorry, I did not understand the previous paragraph. If "when
>> > max_logical_replication_workers is non-zero" then the warning cannot
>> > be emitted inappropriately because that warning is guarded by "if
>> > (max_logical_replication_workers == 0)" (??).
>>
>> You're right, I misunderstood that part. Sorry about that.
>>
>> > > I placed the logic in subscriptioncmds.c so that the warning message can
>> > > better reflect the actual situation, while keeping consistency with
>> > > existing messages such as "subscription was created, but is not
>> > > connected".
>> >
>> > In hindsgght your original patch is very similar to what I posted. I
>> > agree, your messages are better because they are more specific. But
>> > there are multiple calls to ApplyLauncherWakeup() -- not just those 2
>> > that you handled - so I was just unsure if there are some remaining
>> > holes.
>>
>> I see your point.
>>
>> There are other places where ApplyLauncherWakeupAtCommit() is called,
>> for example via AlterSubscription() when retain_dead_tuples is set or
>> when the owner is changed. However, these cases are not affected when
>> max_logical_replication_workers = 0 unless the subscription is enabled.
>>
>> For now, I’ll keep the current approach.
>>
>> > >
>> > > > >
>> > > > > Besides, I also think it would make more sense to issue a warning if the
>> > > > > subscription has no remaining workers to start instead of raising a
>> > > > > warning for 0 setting (the latter seems rare).
>> > > > >
>> > > >
>> > > > It might be rare, but by my understanding, the original post described
>> > > > this specific scenario, whereby the user had previously deliberately
>> > > > configured `max_logical_replication_workers` to 0. Then, some time
>> > > > later, when they attempted CREATE/ALTER SUBSCRIPTION, nothing
>> > > > happened, and there was only silence. If they'd forgotten about their
>> > > > `max_logical_replication_workers` setting, then it could be confusing
>> > > > why nothing was happening.
>> > > >
>> > > > OTOH, when max_logical_replication_workers > 0, then the logical
>> > > > replication launcher would be running, and in that case, there are
>> > > > already plenty of warning logs about not enough worker resources.
>> > >
>> > > Yes. In this case, warnings are emitted to the server log, but they do not
>> > > appear as a response to CREATE/ALTER SUBSCRIPTION. There is an option to
>> > > also emit such warnings during the CREATE/ALTER SUBSCRIPTION command, so
>> > > I will update the patch accordingly.
>> > >
>> > > Nevertheless, this seems to be a different situation from the case where
>> > > logical replication is disabled entirely, so I think the warning messages
>> > > should be handled separately.
>> >
>> > +1. I also think that running out of worker resources is a different
>> > scenario from the entire logical replication being disabled, so it
>> > should be handled separately
>>
>> I've investigated whether we can emit a warning during CREATE/ALTER SUBSCRIPTION
>> when there are no available worker slots to start the subscription.
>>
> I find there is another more warning worthy scenario that should be covered too, what if everything is working normally and then someone changes max_logical_replication_workers to 0 in conf file and restarts, now logical replication fails to continue. In this case since no CREATE or ALTER subscription command was issued, the changes discussed upthread would not produce a warning. Shouldn't we cover this case also?
>>
>> One issue is that, in the current implementation, this condition is only checked
>> when the launcher actually forks a worker. There is no infrastructure to perform
>> this check in advance from a backend process.
>>
>> Adding such infrastructure would likely require acquiring an LWLock (at least in
>> shared mode) to inspect the current worker state. Moreover, even if we implement
>> such a mechanism, the situation may change between the time of the check and the
>> actual worker startup.
>
> What about an alternate way to overcome this, following the same pattern as in apply_error_count for pg_stat_subscription_stats.
> So, a rough sketch of the approach is like this.. Create a new field in PgStat_StatSubEntry say worker_failure_count and add a function to increment this field. Now, in logicalrep_worker_launch when it hits worker == NULL, call this function to increment the worker_failure_count. I have added a rough patch to explain the idea better.
>
> Note that this won't be something as emitting a warning or notice but rather needs to be explicitly queried.
>>
>>
>> Given these limitations, I'm wondering whether introducing this kind of warning
>> is worthwhile.
>>
> --
> Regards,
> Rafia Sabih
> CYBERTEC PostgreSQL International GmbH