On Tue, Oct 20, 2026 at 12:20 AM Amit Kapila <[email protected]> wrote:
>
> On Tue, Oct 20, 2026 at 2:47 AM Masahiko Sawada <[email protected]> wrote:
> >
> > On Sun, Oct 18, 2026 at 5:14 PM shihao zhong <[email protected]> wrote:
> >
> > TBH I'm still unsure that the 0001 patch is the correct approach.
> > ProcessProcSignalBarrier() is written on the assumption that a handler
> > can raise an error. That's why it has a PG_TRY block for a later
> > retry. In v19 we have only two handlers, ProcessBarrierSmgrRelease()
> > and ProcessBarrierUpdateXLogLogicalInfo(), and neither raises an
> > error. But on master, AbsorbDataChecksumsBarrier() raises an error on
> > purpose, with a comment saying it "will be caught by the procsignal
> > machinery for a later retry". With 0001 that error would terminate the
> > session instead of being retried. So the assumption that 0001 adds is
> > already false on master.
> >
> > Other possible solutions are: (1) send the signal barrier at promotion
> > only when we really need to send it. In other words, if we know that
> > logical decoding has been enabled from the startup and no change
> > happened during the recovery, we can skip it even if wal_level =
> > replica. (2) the startup process terminates the backend who doesn't
> > absorb the barrier, like as we resolve the recovery conflicts.
> >
> > (1) would narrow further the case where we need the wait, but doesn't
> > eliminate the case completely.
> >
>
> Right, for example, If the first logical slot appeared on the primary
> shortly before promotion, replay toggled the status (false→true), the
> case counts as "changed," and the wait remains. So, the same problem
> still exists.
>
> > (2) sounds more promising to me, and we
> > already have similar machinery such as
> > ResolveRecoveryConflictWithVirtualXIDs().
> >
>
> Like ResolveRecoveryConflictWithVirtualXIDs(), we may want to wait for
> sometime (which needs GUC or a fixed time wait) before terminating the
> backend. I think this is a direction worth pursuing but I feel it is
> too late for v19 considering the amount/complexity of changes
> required. I feel for v19, we can add a Note in the docs with a
> workaround and then discuss the more robust solution for HEAD.
Thank you for the comments.
shihao zhong drafted (1) the idea above upthread[1]. There seems to be
room for improvement there and the patch would look like the attached:
it initializes the status in shared memory from the control file, so
that the processes the postmaster forks before the startup process
reads the checkpoint record don't start with a stale cache. Given the
patch and analysis[2] shared by Manu, let me summarize the issue and
solutions.
With the v6 patch, we would need to wait for the signal barrier to be
absorbed only when the startup process enables logical decoding at the
end of recovery, which is a rare case. On standbys the status is
toggled by replaying an XLOG_LOGICAL_DECODING_STATUS_CHANGE record,
and the enable case waits for all processes. In some corner cases, the
startup process enables logical decoding at the end of recovery on
behalf of a backend creating a logical slot, so we need a wait for
that case. While making this case rarer is appealing, the v6 patch
doesn't remove it.
The v6 patch also doesn't resolve the case where the replay itself
stops because a backend is stuck writing to a client. As Amit
mentioned, if that delay happens shortly before the promotion, the
promotion would get blocked. The same can be applied to recovery
conflict cases: Manu measured that such a backend is not released by a
recovery conflict signal either[2], since
ProcessClientWriteInterrupt() services only ProcDiePending, and
therefore the recovery gets blocked even with
max_standby_streaming_delay set.
So I think that the recovery (and the promotion) getting stuck due to
client write I/O is not new in v19. max_standby_streaming_delay
doesn't work against a backend stuck writing to a client, and v19 just
adds one more way to hit the same limitation. I still prefer dealing
with this issue only for master, and leave the code in v19 as it is.
For v19 I think we should add a note to the documentation, as Amit
suggested. And possibly we can backpatch the v6 patch in a later minor
release once we test the approach enough on the master.
Regards,
[1]
https://www.postgresql.org/message-id/CAGRkXqR8Kfdd%3DG1Le%3DRzhRHzZBbLtPc44207bwhrPDu_C5ZMEA%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/179247119467.2410654.17218914322784616346%40gmail.com
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 8636ad9ab2c..4ccbb413445 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5513,6 +5513,17 @@ GetActiveWalLevelOnStandby(void)
return ControlFile->wal_level;
}
+/*
+ * Get the logical decoding status recorded in the control file. Returns false
+ * in bootstrap mode, where no control file has been read yet.
+ */
+bool
+GetControlFileLogicalDecodingStatus(void)
+{
+ return ControlFile != NULL &&
+ ControlFile->checkPointCopy.logicalDecodingEnabled;
+}
+
/*
* Register shared memory for XLOG.
*/
diff --git a/src/backend/replication/logical/logicalctl.c b/src/backend/replication/logical/logicalctl.c
index 182d15e5653..22a9d609c52 100644
--- a/src/backend/replication/logical/logicalctl.c
+++ b/src/backend/replication/logical/logicalctl.c
@@ -63,6 +63,7 @@
#include "postgres.h"
+#include "access/xlog.h"
#include "access/xloginsert.h"
#include "access/xlogrecovery.h"
#include "catalog/pg_control.h"
@@ -158,8 +159,15 @@ LogicalDecodingCtlShmemRequest(void *arg)
static void
LogicalDecodingCtlShmemInit(void *arg)
{
- LogicalDecodingCtl->xlog_logical_info = false;
- LogicalDecodingCtl->logical_decoding_enabled = false;
+ /*
+ * Initialize the status with the one recorded in the control file. The
+ * postmaster forks auxiliary processes before the startup process reads
+ * the checkpoint record, and they cache the status at their startup.
+ * StartupLogicalDecodingStatus() corrects it.
+ */
+ LogicalDecodingCtl->xlog_logical_info = GetControlFileLogicalDecodingStatus();
+ LogicalDecodingCtl->logical_decoding_enabled =
+ LogicalDecodingCtl->xlog_logical_info;
LogicalDecodingCtl->pending_disable = false;
LogicalDecodingCtl->last_replayed_enable_lsn = InvalidXLogRecPtr;
}
@@ -171,9 +179,14 @@ LogicalDecodingCtlShmemInit(void *arg)
void
StartupLogicalDecodingStatus(bool last_status)
{
+ bool was_enabled;
+
/* Logical decoding is always disabled when 'minimal' WAL level */
if (wal_level == WAL_LEVEL_MINIMAL)
- return;
+ last_status = false;
+
+ /* Remember what the already running processes have cached */
+ was_enabled = LogicalDecodingCtl->xlog_logical_info;
/*
* Set the initial logical decoding status based on the last status. If
@@ -183,6 +196,26 @@ StartupLogicalDecodingStatus(bool last_status)
*/
LogicalDecodingCtl->xlog_logical_info = last_status;
LogicalDecodingCtl->logical_decoding_enabled = last_status;
+
+ /*
+ * The starting checkpoint can disagree with the control file, for example
+ * when recovery starts from a backup label. In that case the already
+ * running processes have a stale cache, so notify them to refresh it. We
+ * wait only if we turn the status on, since a process with a stale
+ * disabled cache would omit the information logical decoding needs from
+ * the WAL records it writes. Only auxiliary processes are running at this
+ * point, so the wait is short. A stale enabled cache only causes extra
+ * logical information in WAL, which is harmless (see
+ * DisableLogicalDecoding()).
+ */
+ if (IsUnderPostmaster && last_status != was_enabled)
+ {
+ uint64 generation;
+
+ generation = EmitProcSignalBarrier(PROCSIGNAL_BARRIER_UPDATE_XLOG_LOGICAL_INFO);
+ if (last_status)
+ WaitForProcSignalBarrier(generation);
+ }
}
/*
@@ -672,6 +705,7 @@ void
UpdateLogicalDecodingStatusEndOfRecovery(void)
{
bool new_status = false;
+ bool status_changed;
Assert(RecoveryInProgress());
@@ -715,7 +749,8 @@ UpdateLogicalDecodingStatusEndOfRecovery(void)
* by the time of the recheck, the backend instead enables logical
* decoding by itself, which is fine after promotion.)
*/
- if (new_status != LogicalDecodingCtl->logical_decoding_enabled)
+ status_changed = (new_status != LogicalDecodingCtl->logical_decoding_enabled);
+ if (status_changed)
{
/*
* Update both the logical decoding status and logical WAL logging
@@ -745,19 +780,26 @@ UpdateLogicalDecodingStatusEndOfRecovery(void)
else
LWLockRelease(LogicalDecodingControlLock);
+ /* Logical decoding is always enabled with wal_level = logical */
+ Assert(wal_level != WAL_LEVEL_LOGICAL || new_status);
+
/*
* With wal_level = logical, XLogLogicalInfoActive() is always true, so no
- * barrier is needed.
+ * barrier is needed. Note that we can still turn the status on here in
+ * that case, for example when the server ran with 'replica' and had no
+ * logical slot before the last shutdown.
*
- * Otherwise, notify all processes to refresh XLogLogicalInfo. Wait only
- * if new_status is true, even if the shared status is unchanged, since
- * recovery may have left stale caches. Writes are not yet allowed, so we
- * need not wait for running transactions to finish.
+ * If we didn't change the status, no process can have a stale cache
+ * without a barrier already pending for it: a process caches the status
+ * at its startup, and every change to the status emits a barrier.
*
- * If new_status is false, stale caches only cause extra logical
- * information in WAL, which is safe (see DisableLogicalDecoding()).
+ * Otherwise, notify all processes to refresh XLogLogicalInfo, and wait if
+ * we turned the status on. Writes are not yet allowed, so we need not
+ * wait for running transactions to finish. Turning the status off needs
+ * no wait, as a stale enabled cache only causes extra logical information
+ * in WAL, which is safe (see DisableLogicalDecoding()).
*/
- if (IsUnderPostmaster && wal_level != WAL_LEVEL_LOGICAL)
+ if (IsUnderPostmaster && wal_level != WAL_LEVEL_LOGICAL && status_changed)
{
uint64 generation;
diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h
index 2bf5714b51e..3ac5262efe2 100644
--- a/src/include/access/xlog.h
+++ b/src/include/access/xlog.h
@@ -282,6 +282,7 @@ extern void BootStrapXLOG(uint32 data_checksum_version);
extern void InitializeWalConsistencyChecking(void);
extern void LocalProcessControlFile(bool reset);
extern WalLevel GetActiveWalLevelOnStandby(void);
+extern bool GetControlFileLogicalDecodingStatus(void);
extern void StartupXLOG(void);
extern void ShutdownXLOG(int code, Datum arg);
extern bool CreateCheckPoint(int flags);
