On Monday, September 11, 2023 6:32 PM vignesh C <vignesh21@gmail.com> wrote:
>
>
> The attached v7 patch has the changes for the same.
Thanks for updating the patch, here are few comments:
1.
+/*
+ * binary_upgrade_sub_replication_origin_advance
+ *
+ * Update the remote_lsn for the subscriber's replication origin.
+ */
+Datum
+binary_upgrade_sub_replication_origin_advance(PG_FUNCTION_ARGS)
+{
Is there any usage apart from pg_upgrade for this function, if not, I think
we'd better move this function to pg_upgrade_support.c. If yes, I think maybe
better to rename it to a general one.
2.
+ * Verify that all subscriptions have a valid remote_lsn and don't contain
+ * any table in srsubstate different than ready ('r').
+ */
+static void
+check_for_subscription_state(ClusterInfo *cluster)
I think we'd better follow the same style of
check_for_isn_and_int8_passing_mismatch() to record the invalid things in a
file.
3.
+ if (fout->dopt->binary_upgrade && fout->remoteVersion >= 100000)
+ {
+ appendPQExpBuffer(query,
+ "SELECT binary_upgrade_create_sub_rel_state('%s', %u, '%c'",
+ subrinfo->dobj.name,
I think we'd better consider using appendStringLiteral or related function for
the dobj.name here to make sure the string convertion is safe.
4.
The following commit message may need update:
"binary_upgrade_create_sub_rel_state SQL function, and also provides an
additional LSN parameter for CREATE SUBSCRIPTION to restore the underlying
replication origin remote LSN. "
I think we have changed to another approach which doesn't provide new parameter
in DDL.
5.
+ /* Fetch the existing tuple. */
+ tup = SearchSysCacheCopy2(SUBSCRIPTIONNAME, MyDatabaseId,
+ CStringGetDatum(subname));
Since we don't modify the tuple here, SearchSysCache2 seems enough.
6.
+ "LEFT JOIN pg_catalog.pg_database d"
+ " ON d.oid = s.subdbid "
+ "WHERE coalesce(remote_lsn, '0/0') = '0/0'");
For the subscriptions that were just created and finished the table sync but
haven't applied any changes, their remote_lsn will also be 0/0. Do we
need to report ERROR in this case ?
Best Regards,
Hou zj
