On Thu, Jul 16, 2026 at 11:36 AM Ajin Cherian <[email protected]> wrote:
>
> On Fri, Jul 10, 2026 at 6:28 PM Shlok Kyal <[email protected]> wrote:
> >
> >
> > Thanks,
> > Shlok Kyal
>
> Thanks for all the comments. Here's an updated patch addressing these
> comments.
>
Thanks Ajin. A few trivial comments:
1)
+ configured to a value greater than or equal to the number of active
+ replication origins present in the old cluster. The new cluster
must contain no replication origins.
Suggestion:
configured to a value greater than or equal to the number of tracked
replication origins present in the old cluster (reflected in
pg_replication_origin_status)
(active -->tracked, link to pg_replication_origin_status at the end)
2)
+ * restoring its remote_lsn. Used by pg_upgrade to preserve replication
+ * origin IDs across the upgrade.
Shall we get rid of 'Used by pg_upgrade to pre..' as it is used by
other flow as well.
3)
+
+ if (remote_lsn != InvalidXLogRecPtr)
+ {
+ Assert(IsBinaryUpgrade);
+ replorigin_advance(roident, remote_lsn, InvalidXLogRecPtr,
+ false /* backward */,
+ false /* WAL log */);
+ }
We can instead add a comment atop Assert here (remove comment atop
function header as suggested in 2)
/*
* The remote_lsn is expected to be valid only during binary upgrade when
* preserving an existing replication origin. For the normal origin
* creation flow, it should always be InvalidXLogRecPtr.
*/
4)
- * preserved during the upgrade. So, even after creating the slot, the
- * upgraded subscriber may be unable to detect conflicts or log relevant
- * commit timestamps and origins when applying changes from the publisher
- * occurred before the upgrade especially if those changes were not
- * replicated.
+ * upgrade. So, even after creating the slot, the upgraded subscriber may
+ * be unable to detect conflicts or log relevant commit timestamps when
+ * applying changes from the publisher occurred before the upgrade
+ * especially if those changes were not replicated.
I think, we should retain 'origins' in above comment as we still can
not log origin info post-upgrade until commit-timestamp is preserved.
5)
# ------------------------------------------------------
# Check that the data inserted to the publisher when the new subscriber is down
# will be replicated once it is started. Also check that the old subscription
# states and relations origins are all preserved, and that the conflict
# detection slot is created.
# ------------------------------------------------------
We can change this existing comment in test in accordance with new
origin-id presrevation test.
6)
# Verify that the subscription related replication origins are
preserved after upgrade.
# Verify that user created replication origins are preserved after upgrade.
Shall we say:
# Verify that subscription replication origins retain their origin IDs
after upgrade.
# Verify that the user-created replication origin retains its origin
ID after upgrade.
thanks
Shveta
