On Friday, October 24, 2025 11:22 PM vignesh C <vignesh21@gmail.com> wrote: > > On Thu, 23 Oct 2025 at 16:47, Amit Kapila <amit.kapila16@gmail.com> wrote: > > > > On Thu, Oct 23, 2025 at 11:45 AM vignesh C <vignesh21@gmail.com> wrote: > > > > > > The attached patch has the changes for the same. > > > > > > > I have pushed 0001 and the following are comments on 0002. >
One question, I am not sure if this has been discussed before, So while getting sequence information from remote we are also getting the page_lsn of the sequence and we are storing that in pg_subscription_rel. Is it just for the user to see and compare whether the sequence is synced to the latest lsn or is it used for anything else as well? In our patch sert, I don't see much usability information about this field.
Overall patch LGTM, and I really like the idea of getting rid of the hash and converting it into a list, now we don't need to restart the scan unlike hash due to transaction boundary. However I have one more suggestion.
/* + * Establish the connection to the publisher for sequence synchronization. + */ + LogRepWorkerWalRcvConn = + walrcv_connect(MySubscription->conninfo, true, true, + must_use_password, + app_name.data, &err); + if (LogRepWorkerWalRcvConn == NULL) + ereport(ERROR, + errcode(ERRCODE_CONNECTION_FAILURE), + errmsg("sequencesync worker for subscription \"%s\" could not connect to the publisher: %s", + MySubscription->name, err)); + + pfree(app_name.data); + + /* If there are any sequences that need to be copied */ + if (hash_get_num_entries(sequences_to_copy)) + copy_sequences(LogRepWorkerWalRcvConn, subid);
I think we should call 'walrcv_connect' only if we need to copy_sequences right?
