On Mon, Oct 27, 2025 at 10:04 AM Dilip Kumar <[email protected]> wrote:
> On Mon, Oct 27, 2025 at 8:23 AM Zhijie Hou (Fujitsu) < > [email protected]> wrote: > >> On Friday, October 24, 2025 11:22 PM vignesh C <[email protected]> >> wrote: >> > >> > On Thu, 23 Oct 2025 at 16:47, Amit Kapila <[email protected]> >> wrote: >> > > >> > > On Thu, Oct 23, 2025 at 11:45 AM vignesh C <[email protected]> >> 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? -- Regards, Dilip Kumar Google
