public inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedRe: [PATCH] Preserve replication origin OIDs in pg_upgrade
3+ messages / 3 participants
[nested] [flat]
* Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
@ 2026-05-18 10:42 Ajin Cherian <itsajin@gmail.com>
2026-05-22 09:46 ` Re: [PATCH] Preserve replication origin OIDs in pg_upgrade Shlok Kyal <shlok.kyal.oss@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Ajin Cherian @ 2026-05-18 10:42 UTC (permalink / raw)
To: Zsolt Parragi <zsolt.parragi@percona.com>; +Cc: vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Rebased the patch as it was no longer applying.
regards,
Ajin Cherian
Fujitsu Australia
Attachments:
[application/octet-stream] v6-0002-Preserve-replication-origin-OIDs-during-pg_upgrad.patch (22.0K, 2-v6-0002-Preserve-replication-origin-OIDs-during-pg_upgrad.patch)
download | inline diff:
From c03447597e366d13fee22b288a822a66233329a7 Mon Sep 17 00:00:00 2001
From: Ajin Cherian <itsajin@gmail.com>
Date: Mon, 18 May 2026 20:34:44 +1000
Subject: [PATCH v6 2/2] Preserve replication origin OIDs during pg_upgrade
When pg_upgrade migrates a subscriber, replication origin OIDs
(roident) can change across the upgrade. This is a problem because
commit-timestamp records embed roident and are copied directly from
the old cluster's pg_commit_ts directory, causing spurious
"update_origin_differs" conflicts after the upgrade.
Fix this by dumping replication origins as global objects via
pg_dumpall during binary upgrade, using a new function
binary_upgrade_create_replication_origin(oid, name, lsn) to recreate
each origin with its preserved roident and remote_lsn. To avoid
conflicts with this, CreateSubscription() skips replorigin_create()
in binary-upgrade mode since the origin is already created by the
time the subscription is restored.
Author: Ajin Cherian <itsajin@gmail.com>
Reviewer: Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>
Reviewer: Zsolt Parragi <zsolt.parragi@percona.com>
---
src/backend/commands/subscriptioncmds.c | 11 +-
src/backend/utils/adt/pg_upgrade_support.c | 159 +++++++++++++++------
src/bin/pg_dump/pg_dump.c | 38 ++---
src/bin/pg_dump/pg_dumpall.c | 64 +++++++++
src/bin/pg_upgrade/check.c | 13 +-
src/bin/pg_upgrade/info.c | 9 ++
src/bin/pg_upgrade/pg_upgrade.h | 1 +
src/bin/pg_upgrade/t/004_subscription.pl | 42 +++++-
src/include/catalog/pg_proc.dat | 8 +-
9 files changed, 253 insertions(+), 92 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 7c1f05a5fd5..b31e15256a2 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -19,6 +19,7 @@
#include "access/table.h"
#include "access/twophase.h"
#include "access/xact.h"
+#include "catalog/binary_upgrade.h"
#include "catalog/catalog.h"
#include "catalog/dependency.h"
#include "catalog/indexing.h"
@@ -867,9 +868,15 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
* apply workers initialization, and to handle origin creation dynamically
* when tables are added to the subscription. It is not clear whether
* preventing creation of origins is worth additional complexity.
+ *
+ * In binary-upgrade mode, skip origin creation here. This is required to
+ * preserve the roident from the old cluster for this subscription.
*/
- ReplicationOriginNameForLogicalRep(subid, InvalidOid, originname, sizeof(originname));
- replorigin_create(originname);
+ if (!IsBinaryUpgrade)
+ {
+ ReplicationOriginNameForLogicalRep(subid, InvalidOid, originname, sizeof(originname));
+ replorigin_create(originname);
+ }
/*
* Connect to remote side to execute requested commands and fetch table
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index 59c3e7f0146..41fde3f09fc 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -11,10 +11,13 @@
#include "postgres.h"
+#include "access/genam.h"
#include "access/relation.h"
+#include "access/skey.h"
#include "access/table.h"
#include "catalog/binary_upgrade.h"
#include "catalog/heap.h"
+#include "catalog/indexing.h"
#include "catalog/namespace.h"
#include "catalog/pg_subscription_rel.h"
#include "catalog/pg_type.h"
@@ -27,8 +30,10 @@
#include "storage/lmgr.h"
#include "utils/array.h"
#include "utils/builtins.h"
+#include "utils/fmgroids.h"
#include "utils/lsyscache.h"
#include "utils/pg_lsn.h"
+#include "utils/snapmgr.h"
#define CHECK_IS_BINARY_UPGRADE \
@@ -377,71 +382,133 @@ binary_upgrade_add_sub_rel_state(PG_FUNCTION_ARGS)
}
/*
- * binary_upgrade_replorigin_advance
+ * binary_upgrade_create_conflict_detection_slot
*
- * Update the remote_lsn for the subscriber's replication origin.
+ * Create a replication slot to retain information necessary for conflict
+ * detection such as dead tuples, commit timestamps, and origins.
*/
Datum
-binary_upgrade_replorigin_advance(PG_FUNCTION_ARGS)
+binary_upgrade_create_conflict_detection_slot(PG_FUNCTION_ARGS)
{
- Relation rel;
- Oid subid;
- char *subname;
- char originname[NAMEDATALEN];
- ReplOriginId node;
- XLogRecPtr remote_commit;
-
CHECK_IS_BINARY_UPGRADE;
- /*
- * We must ensure a non-NULL subscription name before dereferencing the
- * arguments.
- */
- if (PG_ARGISNULL(0))
- elog(ERROR, "null argument to binary_upgrade_replorigin_advance is not allowed");
-
- subname = text_to_cstring(PG_GETARG_TEXT_PP(0));
- remote_commit = PG_ARGISNULL(1) ? InvalidXLogRecPtr : PG_GETARG_LSN(1);
-
- rel = table_open(SubscriptionRelationId, RowExclusiveLock);
- subid = get_subscription_oid(subname, false);
-
- ReplicationOriginNameForLogicalRep(subid, InvalidOid, originname, sizeof(originname));
-
- /* Lock to prevent the replication origin from vanishing */
- LockRelationOid(ReplicationOriginRelationId, RowExclusiveLock);
- node = replorigin_by_name(originname, false);
-
- /*
- * The server will be stopped after setting up the objects in the new
- * cluster and the origins will be flushed during the shutdown checkpoint.
- * This will ensure that the latest LSN values for origin will be
- * available after the upgrade.
- */
- replorigin_advance(node, remote_commit, InvalidXLogRecPtr,
- false /* backward */ ,
- false /* WAL log */ );
+ CreateConflictDetectionSlot();
- UnlockRelationOid(ReplicationOriginRelationId, RowExclusiveLock);
- table_close(rel, RowExclusiveLock);
+ ReplicationSlotRelease();
PG_RETURN_VOID();
}
/*
- * binary_upgrade_create_conflict_detection_slot
+ * binary_upgrade_create_replication_origin
*
- * Create a replication slot to retain information necessary for conflict
- * detection such as dead tuples, commit timestamps, and origins.
+ * Create a replication origin with a specific OID and name, optionally
+ * restoring its remote_lsn. Used by pg_upgrade to preserve replication
+ * origin OIDs across the upgrade.
*/
Datum
-binary_upgrade_create_conflict_detection_slot(PG_FUNCTION_ARGS)
+binary_upgrade_create_replication_origin(PG_FUNCTION_ARGS)
{
+ Oid node_oid;
+ ReplOriginId node;
+ Name originname;
+ Relation rel;
+ HeapTuple tuple;
+ Datum roname_d;
+ SysScanDesc scan;
+ ScanKeyData key;
+ bool nulls[Natts_pg_replication_origin];
+ Datum values[Natts_pg_replication_origin];
+ bool collides;
+
CHECK_IS_BINARY_UPGRADE;
- CreateConflictDetectionSlot();
+ if (PG_ARGISNULL(0) || PG_ARGISNULL(1))
+ elog(ERROR,
+ "null argument to binary_upgrade_create_replication_origin is not allowed");
+
+ node_oid = PG_GETARG_OID(0);
+
+ if (node_oid == InvalidOid || node_oid > PG_UINT16_MAX)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("replication origin ID %u is out of range", node_oid)));
+
+ node = (ReplOriginId) node_oid;
+ originname = PG_GETARG_NAME(1);
+
+ if (strlen(NameStr(*originname)) > MAX_RONAME_LEN)
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("replication origin name is too long"),
+ errdetail("Replication origin names must be no longer than %d bytes.",
+ MAX_RONAME_LEN)));
+
+ roname_d = CStringGetTextDatum(NameStr(*originname));
+
+ Assert(IsTransactionState());
+
+ rel = table_open(ReplicationOriginRelationId, RowExclusiveLock);
+
+ Assert(!OidIsValid(rel->rd_rel->reltoastrelid));
+
+ /* Check for OID collision */
+ ScanKeyInit(&key,
+ Anum_pg_replication_origin_roident,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(node));
+ scan = systable_beginscan(rel, ReplicationOriginIdentIndex,
+ true /* indexOK */,
+ SnapshotSelf,
+ 1, &key);
+ collides = HeapTupleIsValid(systable_getnext(scan));
+ systable_endscan(scan);
+
+ if (collides)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("replication origin with ID %u already exists", node_oid)));
+
+ /* Check for name collision */
+ ScanKeyInit(&key,
+ Anum_pg_replication_origin_roname,
+ BTEqualStrategyNumber, F_TEXTEQ,
+ roname_d);
+ scan = systable_beginscan(rel, ReplicationOriginNameIndex,
+ true /* indexOK */,
+ SnapshotSelf,
+ 1, &key);
+ collides = HeapTupleIsValid(systable_getnext(scan));
+ systable_endscan(scan);
+
+ if (collides)
+ ereport(ERROR,
+ (errcode(ERRCODE_DUPLICATE_OBJECT),
+ errmsg("replication origin \"%s\" already exists",
+ NameStr(*originname))));
+
+ memset(&nulls, 0, sizeof(nulls));
+ memset(&values, 0, sizeof(values));
+
+ values[Anum_pg_replication_origin_roident - 1] = ObjectIdGetDatum(node);
+ values[Anum_pg_replication_origin_roname - 1] = roname_d;
+
+ tuple = heap_form_tuple(RelationGetDescr(rel), values, nulls);
+ CatalogTupleInsert(rel, tuple);
+ heap_freetuple(tuple);
+ CommandCounterIncrement();
+
+ /* Restore the remote_lsn if provided, while still holding the lock */
+ if (!PG_ARGISNULL(2))
+ {
+ XLogRecPtr remote_commit = PG_GETARG_LSN(2);
- ReplicationSlotRelease();
+ replorigin_advance(node, remote_commit, InvalidXLogRecPtr,
+ false /* backward */,
+ false /* WAL log */);
+ }
+
+ table_close(rel, RowExclusiveLock);
PG_RETURN_VOID();
}
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 452d0b5e98a..a5fb2b42c3d 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5668,37 +5668,15 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
* In binary-upgrade mode, we allow the replication to continue after the
* upgrade.
*/
- if (dopt->binary_upgrade && fout->remoteVersion >= 170000)
+ if (dopt->binary_upgrade && subinfo->subenabled && fout->remoteVersion >= 170000)
{
- if (subinfo->suboriginremotelsn)
- {
- /*
- * Preserve the remote_lsn for the subscriber's replication
- * origin. This value is required to start the replication from
- * the position before the upgrade. This value will be stale if
- * the publisher gets upgraded before the subscriber node.
- * However, this shouldn't be a problem as the upgrade of the
- * publisher ensures that all the transactions were replicated
- * before upgrading it.
- */
- appendPQExpBufferStr(query,
- "\n-- For binary upgrade, must preserve the remote_lsn for the subscriber's replication origin.\n");
- appendPQExpBufferStr(query,
- "SELECT pg_catalog.binary_upgrade_replorigin_advance(");
- appendStringLiteralAH(query, subinfo->dobj.name, fout);
- appendPQExpBuffer(query, ", '%s');\n", subinfo->suboriginremotelsn);
- }
-
- if (subinfo->subenabled)
- {
- /*
- * Enable the subscription to allow the replication to continue
- * after the upgrade.
- */
- appendPQExpBufferStr(query,
- "\n-- For binary upgrade, must preserve the subscriber's running state.\n");
- appendPQExpBuffer(query, "ALTER SUBSCRIPTION %s ENABLE;\n", qsubname);
- }
+ /*
+ * Enable the subscription to allow the replication to continue
+ * after the upgrade.
+ */
+ appendPQExpBufferStr(query,
+ "\n-- For binary upgrade, must preserve the subscriber's running state.\n");
+ appendPQExpBuffer(query, "ALTER SUBSCRIPTION %s ENABLE;\n", qsubname);
}
if (subinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index c1f43113c53..e16918feccf 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -25,6 +25,7 @@
#include <time.h>
#include <unistd.h>
+#include "access/xlogdefs.h"
#include "catalog/pg_authid_d.h"
#include "common/connect.h"
#include "common/file_perm.h"
@@ -76,6 +77,7 @@ static void dropDBs(PGconn *conn);
static void dumpUserConfig(PGconn *conn, const char *username);
static void dumpDatabases(PGconn *conn);
static void dumpTimestamp(const char *msg);
+static void dumpReplicationOrigins(PGconn *conn);
static int runPgDump(const char *dbname, const char *create_opts, char *dbfile);
static void buildShSecLabels(PGconn *conn,
const char *catalog_name, Oid objectId,
@@ -813,6 +815,10 @@ main(int argc, char *argv[])
/* Dump role GUC privileges */
if (server_version >= 150000 && !skip_acls)
dumpRoleGUCPrivs(conn);
+
+ /* Dump replication origins */
+ if (server_version >= 170000 && binary_upgrade && archDumpFormat == archNull)
+ dumpReplicationOrigins(conn);
}
/* Dump tablespaces */
@@ -2339,6 +2345,64 @@ dumpTimestamp(const char *msg)
fprintf(OPF, "-- %s %s\n\n", msg, buf);
}
+static void
+dumpReplicationOrigins(PGconn *conn)
+{
+ PQExpBuffer buf = createPQExpBuffer();
+ PGresult *res;
+ int i_roident;
+ int i_roname;
+ int i_remotelsn;
+
+ /* Get replication origins from catalogs */
+ appendPQExpBufferStr(buf,
+ "SELECT o.*, os.remote_lsn "
+ "FROM pg_catalog.pg_replication_origin o "
+ "LEFT OUTER JOIN pg_catalog.pg_replication_origin_status os ON o.roident = os.local_id ");
+
+ res = executeQuery(conn, buf->data);
+
+ i_roident = PQfnumber(res, "roident");
+ i_roname = PQfnumber(res, "roname");
+ i_remotelsn = PQfnumber(res, "remote_lsn");
+
+ if (PQntuples(res) > 0)
+ fprintf(OPF, "--\n-- Replication Origins \n--\n\n");
+
+ for (int i = 0; i < PQntuples(res); i++)
+ {
+ ReplOriginId roident;
+ const char *roname;
+
+ roident = atooid(PQgetvalue(res, i, i_roident));
+ roname = PQgetvalue(res, i, i_roname);
+
+ resetPQExpBuffer(buf);
+
+ appendPQExpBufferStr(buf, "\n-- For binary upgrade, must preserve replication origin roident and remote_lsn\n");
+ appendPQExpBuffer(buf,
+ "SELECT pg_catalog.binary_upgrade_create_replication_origin("
+ "'%u'::pg_catalog.oid, ", roident);
+ appendStringLiteralConn(buf, roname, conn);
+ appendPQExpBufferStr(buf, "::pg_catalog.name");
+
+ if (!PQgetisnull(res, i, i_remotelsn))
+ {
+ appendPQExpBufferStr(buf, ", ");
+ appendStringLiteralConn(buf, PQgetvalue(res, i, i_remotelsn), conn);
+ appendPQExpBufferStr(buf, "::pg_catalog.pg_lsn");
+ }
+ else
+ appendPQExpBufferStr(buf, ", NULL");
+
+ appendPQExpBufferStr(buf, ");\n");
+ fprintf(OPF, "%s", buf->data);
+ }
+
+ PQclear(res);
+ destroyPQExpBuffer(buf);
+}
+
/*
* read_dumpall_filters - retrieve database identifier patterns from file
*
diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index f5c93e611d2..a3c027c7cd0 100644
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -2306,8 +2306,7 @@ check_new_cluster_replication_slots(void)
* check_new_cluster_subscription_configuration()
*
* Verify that the max_active_replication_origins configuration specified is
- * enough for creating the subscriptions. This is required to create the
- * replication origin for each subscription.
+ * enough for creating all the replication origins.
*/
static void
check_new_cluster_subscription_configuration(void)
@@ -2320,8 +2319,8 @@ check_new_cluster_subscription_configuration(void)
if (GET_MAJOR_VERSION(old_cluster.major_version) < 1700)
return;
- /* Quick return if there are no subscriptions to be migrated. */
- if (old_cluster.nsubs == 0)
+ /* Quick return if there are no replication origins to be migrated. */
+ if (old_cluster.nrepl_origins == 0)
return;
prep_status("Checking new cluster configuration for subscriptions");
@@ -2335,10 +2334,10 @@ check_new_cluster_subscription_configuration(void)
pg_fatal("could not determine parameter settings on new cluster");
max_active_replication_origins = atoi(PQgetvalue(res, 0, 0));
- if (old_cluster.nsubs > max_active_replication_origins)
+ if (old_cluster.nrepl_origins > max_active_replication_origins)
pg_fatal("\"max_active_replication_origins\" (%d) must be greater than or equal to the number of "
- "subscriptions (%d) on the old cluster",
- max_active_replication_origins, old_cluster.nsubs);
+ "replication origins (%d) on the old cluster",
+ max_active_replication_origins, old_cluster.nrepl_origins);
PQclear(res);
PQfinish(conn);
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
index 37fff93892f..630f3f06e24 100644
--- a/src/bin/pg_upgrade/info.c
+++ b/src/bin/pg_upgrade/info.c
@@ -843,6 +843,7 @@ get_subscription_info(ClusterInfo *cluster)
PGconn *conn;
PGresult *res;
int i_nsub;
+ int i_nrepl_origins;
int i_retain_dead_tuples;
conn = connectToServer(cluster, "template1");
@@ -862,6 +863,14 @@ get_subscription_info(ClusterInfo *cluster)
cluster->sub_retain_dead_tuples = (strcmp(PQgetvalue(res, 0, i_retain_dead_tuples), "t") == 0);
PQclear(res);
+
+ res = executeQueryOrDie(conn,
+ "SELECT count(*) AS nrepl_origins "
+ "FROM pg_catalog.pg_replication_origin");
+ i_nrepl_origins = PQfnumber(res, "nrepl_origins");
+ cluster->nrepl_origins = atoi(PQgetvalue(res, 0, i_nrepl_origins));
+ PQclear(res);
+
PQfinish(conn);
}
diff --git a/src/bin/pg_upgrade/pg_upgrade.h b/src/bin/pg_upgrade/pg_upgrade.h
index ccd1ac0d013..77e7ca1b4cd 100644
--- a/src/bin/pg_upgrade/pg_upgrade.h
+++ b/src/bin/pg_upgrade/pg_upgrade.h
@@ -311,6 +311,7 @@ typedef struct
int num_tablespaces;
const char *tablespace_suffix; /* directory specification */
int nsubs; /* number of subscriptions */
+ int nrepl_origins; /* number of replication origins */
bool sub_retain_dead_tuples; /* whether a subscription enables
* retain_dead_tuples. */
} ClusterInfo;
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl b/src/bin/pg_upgrade/t/004_subscription.pl
index 646767f2a65..e8b11d39dd0 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -42,7 +42,7 @@ my $connstr = $publisher->connstr . ' dbname=postgres';
# ------------------------------------------------------
# Check that pg_upgrade fails when max_active_replication_origins configured
-# in the new cluster is less than the number of subscriptions in the old
+# in the new cluster is less than the number of replication origins in the old
# cluster.
# ------------------------------------------------------
# It is sufficient to use disabled subscription to test upgrade failure.
@@ -74,7 +74,7 @@ command_checks_all(
],
1,
[
- qr/"max_active_replication_origins" \(0\) must be greater than or equal to the number of subscriptions \(1\) on the old cluster/
+ qr/"max_active_replication_origins" \(0\) must be greater than or equal to the number of replication origins \(1\) on the old cluster/
],
[qr//],
'run of pg_upgrade where the new cluster has insufficient max_active_replication_origins'
@@ -301,8 +301,30 @@ is($result, qq(t), "Check that the table is in init state");
# Get the replication origin's remote_lsn of the old subscriber
my $remote_lsn = $old_sub->safe_psql('postgres',
- "SELECT remote_lsn FROM pg_replication_origin_status os, pg_subscription s WHERE os.external_id = 'pg_' || s.oid AND s.subname = 'regress_sub4'"
+ "SELECT os.remote_lsn
+ FROM pg_replication_origin_status os
+ JOIN pg_replication_origin o ON o.roident = os.local_id
+ JOIN pg_subscription s ON o.roname = 'pg_' || s.oid::text
+ WHERE s.subname = 'regress_sub4'"
);
+
+# Get the replication origin OIDs (roident) for all subscriptions, keyed by
+# subscription name (which is stable across upgrade, unlike suboid). These
+# must be preserved after upgrade. A mismatch would cause spurious
+# update_origin_differs conflicts.
+my %pre_upgrade_roident;
+my $roident_rows = $old_sub->safe_psql('postgres',
+ "SELECT s.subname, o.roident
+ FROM pg_subscription s
+ JOIN pg_replication_origin o ON o.roname = 'pg_' || s.oid::text
+ ORDER BY s.subname"
+);
+for my $row (split /\n/, $roident_rows)
+{
+ my ($subname, $roident) = split /\|/, $row;
+ $pre_upgrade_roident{$subname} = $roident;
+}
+
# Have the subscription in disabled state before upgrade
$old_sub->safe_psql('postgres', "ALTER SUBSCRIPTION regress_sub5 DISABLE");
@@ -378,6 +400,20 @@ regress_sub5|f|f|f),
"check that the subscription's running status, failover, and retain_dead_tuples are preserved"
);
+# Verify that replication origin OIDs are preserved after upgrade.
+my $post_roident_rows = $new_sub->safe_psql('postgres',
+ "SELECT s.subname, o.roident
+ FROM pg_subscription s
+ JOIN pg_replication_origin o ON o.roname = 'pg_' || s.oid::text
+ ORDER BY s.subname"
+);
+for my $row (split /\n/, $post_roident_rows)
+{
+ my ($subname, $roident) = split /\|/, $row;
+ is($roident, $pre_upgrade_roident{$subname},
+ "roident preserved for subscription '$subname' 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/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 3a28406981d..1d2d624392e 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11960,10 +11960,6 @@
provolatile => 'v', proparallel => 'u', prorettype => 'void',
proargtypes => 'text oid char pg_lsn',
prosrc => 'binary_upgrade_add_sub_rel_state' },
-{ oid => '6320', descr => 'for use by pg_upgrade (remote_lsn for origin)',
- proname => 'binary_upgrade_replorigin_advance', proisstrict => 'f',
- provolatile => 'v', proparallel => 'u', prorettype => 'void',
- proargtypes => 'text pg_lsn', prosrc => 'binary_upgrade_replorigin_advance' },
{ oid => '6505', descr => 'for use by pg_upgrade (conflict detection slot)',
proname => 'binary_upgrade_create_conflict_detection_slot',
proisstrict => 'f', provolatile => 'v', proparallel => 'u',
@@ -11973,6 +11969,10 @@
proname => 'binary_upgrade_set_next_pg_subscription_oid', provolatile => 'v',
proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
prosrc => 'binary_upgrade_set_next_pg_subscription_oid' },
+{ oid => '9161', descr => 'for use by pg_upgrade (replication origin)',
+ proname => 'binary_upgrade_create_replication_origin', proisstrict => 'f',
+ provolatile => 'v', proparallel => 'u', prorettype => 'void',
+ proargtypes => 'oid name pg_lsn', prosrc => 'binary_upgrade_create_replication_origin' },
# conversion functions
{ oid => '4310', descr => 'internal conversion function for KOI8R to WIN1251',
--
2.47.3
[application/octet-stream] v6-0001-Preserve-subscription-OIDs-during-pg_upgrade.patch (9.1K, 3-v6-0001-Preserve-subscription-OIDs-during-pg_upgrade.patch)
download | inline diff:
From 9be4a488196d12cac3ccaba132b0efcef14a7a87 Mon Sep 17 00:00:00 2001
From: Ajin Cherian <itsajin@gmail.com>
Date: Mon, 18 May 2026 18:41:05 +1000
Subject: [PATCH v6 1/2] Preserve subscription OIDs during pg_upgrade
Currently subscription OIDs can be changed when a cluster is upgraded
using pg_upgrade. This is required for a subsequent patch which will
preserve the replication oids after upgrade.
Author: Vignesh C <vignesh21@gmail.com>
---
src/backend/commands/subscriptioncmds.c | 25 +++++++++++++++++--
src/backend/utils/adt/pg_upgrade_support.c | 10 ++++++++
src/bin/pg_dump/pg_dump.c | 8 ++++++
src/bin/pg_upgrade/pg_upgrade.c | 3 +++
src/bin/pg_upgrade/t/004_subscription.pl | 7 ++++++
src/include/catalog/binary_upgrade.h | 1 +
src/include/catalog/pg_proc.dat | 4 +++
.../expected/spgist_name_ops.out | 6 +++--
8 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 523959ba0ce..7c1f05a5fd5 100644
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -83,6 +83,12 @@
/* check if the 'val' has 'bits' set */
#define IsSet(val, bits) (((val) & (bits)) == (bits))
+/*
+ * This will be set by the pg_upgrade_support function --
+ * binary_upgrade_set_next_pg_subscription_oid().
+ */
+Oid binary_upgrade_next_pg_subscription_oid = InvalidOid;
+
/*
* Structure to hold a bitmap representing the user-provided CREATE/ALTER
* SUBSCRIPTION command options and the parsed/default values of each of them.
@@ -772,8 +778,23 @@ CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
memset(values, 0, sizeof(values));
memset(nulls, false, sizeof(nulls));
- subid = GetNewOidWithIndex(rel, SubscriptionObjectIndexId,
- Anum_pg_subscription_oid);
+ /* Use binary-upgrade override for pg_subscription.oid? */
+ if (IsBinaryUpgrade)
+ {
+ if (!OidIsValid(binary_upgrade_next_pg_subscription_oid))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("pg_subscription OID value not set when in binary upgrade mode")));
+
+ subid = binary_upgrade_next_pg_subscription_oid;
+ binary_upgrade_next_pg_subscription_oid = InvalidOid;
+ }
+ else
+ {
+ subid = GetNewOidWithIndex(rel, SubscriptionObjectIndexId,
+ Anum_pg_subscription_oid);
+ }
+
values[Anum_pg_subscription_oid - 1] = ObjectIdGetDatum(subid);
values[Anum_pg_subscription_subdbid - 1] = ObjectIdGetDatum(MyDatabaseId);
values[Anum_pg_subscription_subskiplsn - 1] = LSNGetDatum(InvalidXLogRecPtr);
diff --git a/src/backend/utils/adt/pg_upgrade_support.c b/src/backend/utils/adt/pg_upgrade_support.c
index b505a6b4fee..59c3e7f0146 100644
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -181,6 +181,16 @@ binary_upgrade_set_next_pg_authid_oid(PG_FUNCTION_ARGS)
PG_RETURN_VOID();
}
+Datum
+binary_upgrade_set_next_pg_subscription_oid(PG_FUNCTION_ARGS)
+{
+ Oid subid = PG_GETARG_OID(0);
+
+ CHECK_IS_BINARY_UPGRADE;
+ binary_upgrade_next_pg_subscription_oid = subid;
+ PG_RETURN_VOID();
+}
+
Datum
binary_upgrade_create_empty_extension(PG_FUNCTION_ARGS)
{
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index d56dcc701ce..452d0b5e98a 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -5583,6 +5583,14 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
appendPQExpBuffer(delq, "DROP SUBSCRIPTION %s;\n",
qsubname);
+ if (dopt->binary_upgrade)
+ {
+ appendPQExpBufferStr(query, "\n-- For binary upgrade, must preserve pg_subscription.oid\n");
+ appendPQExpBuffer(query,
+ "SELECT pg_catalog.binary_upgrade_set_next_pg_subscription_oid('%u'::pg_catalog.oid);\n\n",
+ subinfo->dobj.catId.oid);
+ }
+
appendPQExpBuffer(query, "CREATE SUBSCRIPTION %s ",
qsubname);
if (subinfo->subservername)
diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 2127d297bfe..4e853096698 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -35,6 +35,9 @@
*
* We control all assignments of pg_database.oid because we want the directory
* names to match between the old and new cluster.
+ *
+ * We control assignment of pg_subscription.oid because we want the oid to
+ * match between the old and new cluster.
*/
diff --git a/src/bin/pg_upgrade/t/004_subscription.pl b/src/bin/pg_upgrade/t/004_subscription.pl
index c94a82deae0..646767f2a65 100644
--- a/src/bin/pg_upgrade/t/004_subscription.pl
+++ b/src/bin/pg_upgrade/t/004_subscription.pl
@@ -313,6 +313,9 @@ my $tab_upgraded1_oid = $old_sub->safe_psql('postgres',
my $tab_upgraded2_oid = $old_sub->safe_psql('postgres',
"SELECT oid FROM pg_class WHERE relname = 'tab_upgraded2'");
+$sub_oid = $old_sub->safe_psql('postgres',
+ "SELECT oid FROM pg_subscription ORDER BY subname");
+
$old_sub->stop;
# Change configuration so that initial table sync does not get started
@@ -359,6 +362,10 @@ $publisher->safe_psql(
$new_sub->start;
+# The subscription oid should be preserved
+$result = $new_sub->safe_psql('postgres', "SELECT oid FROM pg_subscription ORDER BY subname");
+is($result, qq($sub_oid), "subscription oid should have been preserved");
+
# The subscription's running status, failover option, and retain_dead_tuples
# option should be preserved in the upgraded instance. So regress_sub4 should
# still have subenabled, subfailover, and subretaindeadtuples set to true,
diff --git a/src/include/catalog/binary_upgrade.h b/src/include/catalog/binary_upgrade.h
index 7bf7ae44385..b15b18e7dc9 100644
--- a/src/include/catalog/binary_upgrade.h
+++ b/src/include/catalog/binary_upgrade.h
@@ -32,6 +32,7 @@ extern PGDLLIMPORT RelFileNumber binary_upgrade_next_toast_pg_class_relfilenumbe
extern PGDLLIMPORT Oid binary_upgrade_next_pg_enum_oid;
extern PGDLLIMPORT Oid binary_upgrade_next_pg_authid_oid;
+extern PGDLLIMPORT Oid binary_upgrade_next_pg_subscription_oid;
extern PGDLLIMPORT bool binary_upgrade_record_init_privs;
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index be157a5fbe9..3a28406981d 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -11969,6 +11969,10 @@
proisstrict => 'f', provolatile => 'v', proparallel => 'u',
prorettype => 'void', proargtypes => '',
prosrc => 'binary_upgrade_create_conflict_detection_slot' },
+{ oid => '9160', descr => 'for use by pg_upgrade',
+ proname => 'binary_upgrade_set_next_pg_subscription_oid', provolatile => 'v',
+ proparallel => 'r', prorettype => 'void', proargtypes => 'oid',
+ prosrc => 'binary_upgrade_set_next_pg_subscription_oid' },
# conversion functions
{ oid => '4310', descr => 'internal conversion function for KOI8R to WIN1251',
diff --git a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
index 1ee65ede243..39d43368c42 100644
--- a/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
+++ b/src/test/modules/spgist_name_ops/expected/spgist_name_ops.out
@@ -59,11 +59,12 @@ select * from t
binary_upgrade_set_next_multirange_pg_type_oid | 1 | binary_upgrade_set_next_multirange_pg_type_oid
binary_upgrade_set_next_pg_authid_oid | | binary_upgrade_set_next_pg_authid_oid
binary_upgrade_set_next_pg_enum_oid | | binary_upgrade_set_next_pg_enum_oid
+ binary_upgrade_set_next_pg_subscription_oid | | binary_upgrade_set_next_pg_subscription_oid
binary_upgrade_set_next_pg_tablespace_oid | | binary_upgrade_set_next_pg_tablespace_oid
binary_upgrade_set_next_pg_type_oid | | binary_upgrade_set_next_pg_type_oid
binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid
binary_upgrade_set_next_toast_relfilenode | | binary_upgrade_set_next_toast_relfilenode
-(13 rows)
+(14 rows)
-- Verify clean failure when INCLUDE'd columns result in overlength tuple
-- The error message details are platform-dependent, so show only SQLSTATE
@@ -108,11 +109,12 @@ select * from t
binary_upgrade_set_next_multirange_pg_type_oid | 1 | binary_upgrade_set_next_multirange_pg_type_oid
binary_upgrade_set_next_pg_authid_oid | | binary_upgrade_set_next_pg_authid_oid
binary_upgrade_set_next_pg_enum_oid | | binary_upgrade_set_next_pg_enum_oid
+ binary_upgrade_set_next_pg_subscription_oid | | binary_upgrade_set_next_pg_subscription_oid
binary_upgrade_set_next_pg_tablespace_oid | | binary_upgrade_set_next_pg_tablespace_oid
binary_upgrade_set_next_pg_type_oid | | binary_upgrade_set_next_pg_type_oid
binary_upgrade_set_next_toast_pg_class_oid | 1 | binary_upgrade_set_next_toast_pg_class_oid
binary_upgrade_set_next_toast_relfilenode | | binary_upgrade_set_next_toast_relfilenode
-(13 rows)
+(14 rows)
\set VERBOSITY sqlstate
insert into t values(repeat('xyzzy', 12), 42, repeat('xyzzy', 4000));
--
2.47.3
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
2026-05-18 10:42 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade Ajin Cherian <itsajin@gmail.com>
@ 2026-05-22 09:46 ` Shlok Kyal <shlok.kyal.oss@gmail.com>
2026-05-22 10:27 ` Re: [PATCH] Preserve replication origin OIDs in pg_upgrade shveta malik <shveta.malik@gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Shlok Kyal @ 2026-05-22 09:46 UTC (permalink / raw)
To: Ajin Cherian <itsajin@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Mon, 18 May 2026 at 16:13, Ajin Cherian <itsajin@gmail.com> wrote:
>
> Rebased the patch as it was no longer applying.
>
Hi Ajin,
I have started reviewing the patch. Here is my comment for v6-0002 patch:
Suppose we have a replication setup: publisher -> subscriber
and we are upgrading subscriber to subscriber_new.
And if initially 'subscriber_new' has a replication origin, upgrading
the cluster can error out.
Example:
We set up a logical replication between publisher node and subscriber node.
On subscriber node:
postgres=# SELECT * FROM pg_replication_origin;
roident | roname
---------+----------
1 | pg_16393
(1 row)
And initially subscriber_new has a replication origin:
postgres=# select pg_replication_origin_create('myname');
pg_replication_origin_create
------------------------------
1
(1 row)
postgres=# SELECT * FROM pg_replication_origin;
roident | roname
---------+--------
1 | myname
(1 row)
Now, if we run pg_upgrade to upgrade subscriber node to subscriber_new
node, we get an error:
```
SELECT pg_catalog.binary_upgrade_create_replication_origin('1'::pg_catalog.oid,
'pg_16393'::pg_catalog.name, '0/01743078'::pg_catalog.pg_lsn);
psql:subscriber_new/pg_upgrade_output.d/20260522T140312.807/dump/pg_upgrade_dump_globals.sql:37:
ERROR: replication origin with ID 1 already exists
```
This error occurs in "Performing Upgrade" stage. Should we add a check
in the "Performing Consistency Checks" stage so that we don't need to
re-initdb the new cluster to perform the upgrade?
Maybe we can add a check similar to
check_new_cluster_replication_slots(), where pg_upgrade errors out if
the new cluster already contains replication origins. Thoughts?
Thanks,
Shlok Kyal
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] Preserve replication origin OIDs in pg_upgrade
2026-05-18 10:42 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade Ajin Cherian <itsajin@gmail.com>
2026-05-22 09:46 ` Re: [PATCH] Preserve replication origin OIDs in pg_upgrade Shlok Kyal <shlok.kyal.oss@gmail.com>
@ 2026-05-22 10:27 ` shveta malik <shveta.malik@gmail.com>
0 siblings, 0 replies; 3+ messages in thread
From: shveta malik @ 2026-05-22 10:27 UTC (permalink / raw)
To: Shlok Kyal <shlok.kyal.oss@gmail.com>; Ajin Cherian <itsajin@gmail.com>; +Cc: Zsolt Parragi <zsolt.parragi@percona.com>; vignesh C <vignesh21@gmail.com>; Hayato Kuroda (Fujitsu) <kuroda.hayato@fujitsu.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; shveta malik <shvetamalik@gmail.com>
On Fri, May 22, 2026 at 3:16 PM Shlok Kyal <shlok.kyal.oss@gmail.com> wrote:
>
> On Mon, 18 May 2026 at 16:13, Ajin Cherian <itsajin@gmail.com> wrote:
> >
> > Rebased the patch as it was no longer applying.
> >
> Hi Ajin,
>
> I have started reviewing the patch. Here is my comment for v6-0002 patch:
>
> Suppose we have a replication setup: publisher -> subscriber
> and we are upgrading subscriber to subscriber_new.
> And if initially 'subscriber_new' has a replication origin, upgrading
> the cluster can error out.
>
> Example:
> We set up a logical replication between publisher node and subscriber node.
>
> On subscriber node:
> postgres=# SELECT * FROM pg_replication_origin;
> roident | roname
> ---------+----------
> 1 | pg_16393
> (1 row)
>
> And initially subscriber_new has a replication origin:
> postgres=# select pg_replication_origin_create('myname');
> pg_replication_origin_create
> ------------------------------
> 1
> (1 row)
>
> postgres=# SELECT * FROM pg_replication_origin;
> roident | roname
> ---------+--------
> 1 | myname
> (1 row)
>
> Now, if we run pg_upgrade to upgrade subscriber node to subscriber_new
> node, we get an error:
> ```
> SELECT pg_catalog.binary_upgrade_create_replication_origin('1'::pg_catalog.oid,
> 'pg_16393'::pg_catalog.name, '0/01743078'::pg_catalog.pg_lsn);
> psql:subscriber_new/pg_upgrade_output.d/20260522T140312.807/dump/pg_upgrade_dump_globals.sql:37:
> ERROR: replication origin with ID 1 already exists
> ```
>
> This error occurs in "Performing Upgrade" stage. Should we add a check
> in the "Performing Consistency Checks" stage so that we don't need to
> re-initdb the new cluster to perform the upgrade?
> Maybe we can add a check similar to
> check_new_cluster_replication_slots(), where pg_upgrade errors out if
> the new cluster already contains replication origins. Thoughts?
+1. I had the same thought while reviewing the patch today. We should
have it unless there is a reason we have avoided it??
Few trivial comments:
1)
+#include "access/skey.h"
+#include "catalog/indexing.h"
pg_upgrade_support.c compiles without above.
2)
+ Assert(!OidIsValid(rel->rd_rel->reltoastrelid));
Is there a reason for this sanity check? I generally do not see a
Null-Toast table sanity check after every table_open.
3)
+
+ /* Dump replication origins */
+ if (server_version >= 170000 && binary_upgrade && archDumpFormat == archNull)
+ dumpReplicationOrigins(conn);
why the check is for PG17 specifically?
thanks
Shveta
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-05-22 10:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-05-18 10:42 Re: [PATCH] Preserve replication origin OIDs in pg_upgrade Ajin Cherian <itsajin@gmail.com>
2026-05-22 09:46 ` Shlok Kyal <shlok.kyal.oss@gmail.com>
2026-05-22 10:27 ` shveta malik <shveta.malik@gmail.com>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox