Re: [PATCH] Preserve replication origin OIDs in pg_upgrade

On Wed, Jun 17, 2026 at 4:29 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> On Wed, Jun 17, 2026 at 2:49 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> >
> > I have addressed the comments in v8 (attached). I've also now taken
> > out the check for PG17 on old cluster and now all clusters which have
> > replication origins are migrated.
>
> Hi Ajin, Thanks for the patches.
> I have started reviewing the patches. Please find a few initial
> comments below (from v7 review, still applicable to v8):

Please find a couple more comments for the v8-002 patch:

4) I was able to build successfully without all three new header
inclusions below, so we may not need them.
--- a/src/backend/utils/adt/pg_upgrade_support.c
+++ b/src/backend/utils/adt/pg_upgrade_support.c
@@ -11,6 +11,7 @@
+#include "access/genam.h"
...
+#include "utils/fmgroids.h"
...
+#include "utils/snapmgr.h"
~~~

5) Race condition in origin.c: replorigin_create()
- heap_freetuple(tuple);
+ table_close(rel, ExclusiveLock);
+
+ replorigin_create_with_id(roident, roname, InvalidXLogRecPtr);
+

Due to the above refactoring, CREATE SUBSCRIPTION can now fail because
of a race between two concurrent CREATE SUBSCRIPTION commands:

postgres=# create subscription sub2 connection 'dbname=postgres
host=localhost port=7733' publication pub1;
ERROR: replication origin with ID 2 already exists

I think this happens because the table lock is released, allowing
another session to create a pg_origin with the next available roident,
which replorigin_create_with_id() later attempts to use.

Would it make sense to call replorigin_create_with_id() before
table_close() so the lock is still held? I tested this and it works,
though I'm not fully sure whether self-locking could be a concern
here, since replorigin_create_with_id() again acquires a
RowExclusiveLock on the same table.
~~~

--
Thanks,
Nisha