Hi Ajin,
Thanks -- 0/0 lines up now, and the pg_upgrade.c comment is fixed too. But
`if (IsBinaryUpgrade)` also advances origins that were never tracked.
Old cluster, max_active_replication_origins = 1. The second origin can be
created because it is never tracked:
SELECT pg_replication_origin_create('advanced_one');
SELECT pg_replication_origin_advance('advanced_one', '0/1000');
SELECT pg_replication_origin_create('unadvanced_one');
SELECT * FROM pg_replication_origin;
roident | roname
---------+----------------
1 | advanced_one
2 | unadvanced_one
SELECT * FROM pg_replication_origin_status;
local_id | external_id | remote_lsn | local_lsn
----------+--------------+------------+------------
1 | advanced_one | 0/00001000 | 0/00000000
Upgrading to a new cluster with the same max_active_replication_origins = 1:
Performing Consistency Checks
...
Checking replication origins in new cluster ok
Performing Upgrade
...
Restoring global objects in the new cluster
*failure*
ERROR: could not find free replication state slot for replication origin
with ID 2
The check passed because it counted the one status row; the restore then wanted
two slots. And it gets there after "If pg_upgrade fails after this point, you
must re-initdb the new cluster before continuing.", so a worse landing place
than the refusal I reported on v13.
HEAD guards this in the dumpSubscription() path the patch replaces:
if (subinfo->suboriginremotelsn)
NULL exactly when there was no status row, so HEAD advances iff the origin was
tracked, 0/0 included. dumpReplicationOrigins() still draws the same line --
if (!PQgetisnull(res, i, i_remotelsn))
emits the lsn, and NULL otherwise -- so the distinction survives the dump; it
just gets dropped in the backend, where a missing LSN and a real 0/0 are both
InvalidXLogRecPtr by the time replorigin_create_with_id() sees them. An
untracked origin then starts counting against max_active_replication_origins,
which is what counting status rows was meant to avoid.
The caller still knows which is which -- NULL vs '0/0' -- so passing it down:
void
replorigin_create_with_id(ReplOriginId roident, const char *roname,
- XLogRecPtr remote_lsn, Relation rel)
+ XLogRecPtr remote_lsn, bool need_advance, Relation rel)
@@
- if (IsBinaryUpgrade)
+ if (need_advance)
{
- /* If part of a binary upgrade, advance the origin as well */
+ Assert(IsBinaryUpgrade);
replorigin_advance(roident, remote_lsn, InvalidXLogRecPtr,
@@ replorigin_create()
- replorigin_create_with_id(roident, roname, InvalidXLogRecPtr, rel);
+ replorigin_create_with_id(roident, roname, InvalidXLogRecPtr, false, rel);
@@ binary_upgrade_create_replication_origin()
- replorigin_create_with_id(node, originname, remote_lsn, rel);
+ replorigin_create_with_id(node, originname, remote_lsn,
+ !PG_ARGISNULL(2), rel);
reproduces the old cluster's tracking exactly: the case above upgrades, and an
origin sitting at 0/0 still keeps its status row.
004_subscription.pl already creates a user origin and never advances it, so it
only wanted an assertion that the origin is still untracked afterwards; that
one fails on v14 and passes with the change. Attached is the whole thing, code
and test, on top of v14 -- the pg_upgrade suite and the main regression tests
are green with it.
The other way to close the gap is to leave the advance alone and count all
origins in the check instead. That works too, but it makes pg_upgrade ask for a
max_active_replication_origins the server itself doesn't require -- untracked
origins are free to create today -- so matching the old cluster's tracking
looked like the smaller change.
Thanks,
Rui
From 1c146866989f00f081537f88feba00f3db62067c Mon Sep 17 00:00:00 2001
From: Rui Zhao <[email protected]>
Date: Thu, 30 Jul 2026 14:38:17 +0800
Subject: [PATCH] Only advance replication origins the old cluster was tracking
binary_upgrade_create_replication_origin() advances every origin it
recreates, because replorigin_create_with_id() keys the advance off
IsBinaryUpgrade. An origin that had no pg_replication_origin_status row
on the old cluster arrives here with remote_lsn defaulted to
InvalidXLogRecPtr, indistinguishable from one that was tracked at 0/0, so
it gets advanced too and starts consuming a replication state slot.
check_new_cluster_replication_origins() sizes
max_active_replication_origins against the origins that were tracked on
the old cluster, so those extra slots are unaccounted for and the restore
can run out mid-way, after the point where the new cluster has to be
re-initdb'd.
pg_dumpall already distinguishes the two cases, passing the LSN only when
a status row exists and NULL otherwise. Carry that down to
replorigin_create_with_id() as an explicit flag instead of re-deriving it
from remote_lsn, so the new cluster tracks exactly what the old one did.
---
src/backend/replication/logical/origin.c | 19 +++++++++++++++----
src/backend/utils/adt/pg_upgrade_support.c | 3 ++-
src/bin/pg_upgrade/t/004_subscription.pl | 14 ++++++++++++++
src/include/replication/origin.h | 3 ++-
4 files changed, 33 insertions(+), 6 deletions(-)
diff --git a/src/backend/replication/logical/origin.c
b/src/backend/replication/logical/origin.c
index bbc7d2fbc0..fa629a610a 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -271,13 +271,18 @@ replorigin_by_name(const char *roname, bool missing_ok)
* Create a replication origin with a specific ID and name, optionally
* restoring its remote_lsn.
*
+ * need_advance says whether the origin should also be made tracked, which is
+ * what reserves it a replication state slot. Only binary upgrade sets it, for
+ * origins the old cluster was tracking; remote_lsn alone cannot say, since a
+ * missing status row and a status row at 0/0 both arrive as InvalidXLogRecPtr.
+ *
* Caller must hold an exclusive lock on ReplicationOriginRelationId.
*
* Needs to be called in a transaction.
*/
void
replorigin_create_with_id(ReplOriginId roident, const char *roname,
- XLogRecPtr remote_lsn,
Relation rel)
+ XLogRecPtr remote_lsn, bool
need_advance, Relation rel)
{
Datum roname_d;
bool nulls[Natts_pg_replication_origin];
@@ -300,9 +305,15 @@ replorigin_create_with_id(ReplOriginId roident, const char
*roname,
heap_freetuple(tuple);
CommandCounterIncrement();
- if (IsBinaryUpgrade)
+ /*
+ * During binary upgrade, reproduce the old cluster's tracking state: an
+ * origin that had a pg_replication_origin_status row is advanced here
so
+ * it is tracked on the new cluster too (even at 0/0), while one that
was
+ * never tracked stays untracked and consumes no replication state slot.
+ */
+ if (need_advance)
{
- /* If part of a binary upgrade, advance the origin as well */
+ Assert(IsBinaryUpgrade);
replorigin_advance(roident, remote_lsn, InvalidXLogRecPtr,
false /* backward */,
false /* WAL log */);
@@ -394,7 +405,7 @@ replorigin_create(const char *roname)
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
errmsg("could not find free replication origin
ID")));
- replorigin_create_with_id(roident, roname, InvalidXLogRecPtr, rel);
+ replorigin_create_with_id(roident, roname, InvalidXLogRecPtr, false,
rel);
table_close(rel, ExclusiveLock);
diff --git a/src/backend/utils/adt/pg_upgrade_support.c
b/src/backend/utils/adt/pg_upgrade_support.c
index cea73c40a1..7fd005e128 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -447,7 +447,8 @@ binary_upgrade_create_replication_origin(PG_FUNCTION_ARGS)
/* Acquire an exclusive lock before inserting the new origin. */
rel = table_open(ReplicationOriginRelationId, ExclusiveLock);
- replorigin_create_with_id(node, originname, remote_lsn, rel);
+ replorigin_create_with_id(node, originname, remote_lsn,
+ !PG_ARGISNULL(2),
rel);
table_close(rel, ExclusiveLock);
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl
b/src/bin/pg_upgrade/t/004_subscription.pl
index 6d3a3a41ad..ada3bc299a 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -452,6 +452,20 @@ is($post_user_roident,
$pre_upgrade_roident{$user_origin_name},
"roident preserved for user-created origin '$user_origin_name' after
upgrade"
);
+# The user-created origin above was never advanced, so it has no
+# pg_replication_origin_status row on the old cluster and must not gain one
+# here: an origin that is not tracked consumes no replication state slot, and
+# max_active_replication_origins was only checked against the origins that
+# were tracked before the upgrade.
+$result = $new_sub->safe_psql(
+ 'postgres', qq[
+ SELECT count(*) FROM pg_replication_origin_status s
+ JOIN pg_replication_origin o ON o.roident = s.local_id
+ WHERE o.roname = '$user_origin_name']);
+is($result, qq(0),
+ "never-advanced origin '$user_origin_name' is still untracked after
upgrade"
+);
+
# Subscription relations should be preserved
$result = $new_sub->safe_psql('postgres',
"SELECT srrelid, srsubstate FROM pg_subscription_rel ORDER BY srrelid");
diff --git a/src/include/replication/origin.h b/src/include/replication/origin.h
index 11ee630fb2..08d167be64 100644
--- a/src/include/replication/origin.h
+++ b/src/include/replication/origin.h
@@ -57,7 +57,8 @@ extern PGDLLIMPORT int max_active_replication_origins;
extern ReplOriginId replorigin_by_name(const char *roname, bool missing_ok);
extern ReplOriginId replorigin_create(const char *roname);
extern void replorigin_create_with_id(ReplOriginId roident, const char *roname,
-
XLogRecPtr remote_lsn, Relation rel);
+
XLogRecPtr remote_lsn, bool need_advance,
+
Relation rel);
extern void replorigin_drop_by_name(const char *name, bool missing_ok, bool
nowait);
extern bool replorigin_by_oid(ReplOriginId roident, bool missing_ok,
char **roname);
--
2.43.7
