Thread
-
Re: Logical Replication of sequences
Shlok Kyal <shlok.kyal.oss@gmail.com> — 2025-11-10T09:03:54Z
On Fri, 7 Nov 2025 at 20:18, vignesh C <vignesh21@gmail.com> wrote: > > On Fri, 7 Nov 2025 at 14:54, shveta malik <shveta.malik@gmail.com> wrote: > > > > On Fri, Nov 7, 2025 at 10:58 AM vignesh C <vignesh21@gmail.com> wrote: > > > > > > > > > Thanks for pushing the patch, here is a rebased version of the > > > remaining patches. > > > > > > > Please find a few comments on doc patch: > > > > 1) > > + them. To verify this, compare the > > + <link linkend="catalog-pg-subscription-rel">pg_subscription_rel</link>.<structfield>srsublsn</structfield> > > + on the subscriber with the page_lsn obtained from the > > + <function>pg_get_sequence_data</function> for the sequence on the > > publisher. > > > > Is there a way to give link of 'pg_get_sequence_data' here? > > Modified > > > 2) > > + <warning> > > + <para> > > + Each sequence caches a block of values (typically 32) in memory before > > + generating a new WAL record, so its LSN advances only after the entire > > + cached batch has been consumed. As a result, sequence value > > drift cannot be > > + detected by comparing LSNs for sequence increments that fall within the > > + same cached block. > > + </para> > > + </warning> > > > > In such a case, shall we mention that compare last_value to see the > > drift? Thoughts? > > I was not sure as it might not be very efficient > > > 3) > > > > + To detect this, compare the > > + <link linkend="catalog-pg-subscription-rel">pg_subscription_rel</link>.<structfield>srsublsn</structfield> > > + on the subscriber with the page_lsn obtained from the > > + <function>pg_get_sequence_data</function> for the sequence on the > > publisher. > > > > > > We have mentioned above. But in the example of the same, we do not > > show srsublsn or page_lsn anywhere. Shall we query and show that as > > well? > > Updated example > > > > > 4) > > Maximum number of synchronization workers per subscription. This > > parameter controls the amount of parallelism of the initial data copy > > during the subscription initialization or when new tables are added. > > + One additional worker is also needed for sequence synchronization. > > </para> > > > > Since now the first line is talking only about table-sync, shall we tweak it: > > 'of the initial data copy' --> 'of the initial data copy for tables' > > Modified > > > 5) > > + Returns information about the sequence. <literal>last_value</literal> > > + indicates last sequence value set in sequence by nextval or setval, > > > > last_value can also be set by seq synchronization. Do you think that > > we need to mention that or current info is good enough? > > Updated > > The attached v20251107_2 version patch has the changes for the same. > Hi Vignesh, While working on another thread, I found that in HEAD gram.y has grammar which was committed as part of this thread: ``` | CREATE PUBLICATION name FOR pub_obj_type_list opt_definition { CreatePublicationStmt *n = makeNode(CreatePublicationStmt); n->pubname = $3; n->pubobjects = (List *) $5; preprocess_pub_all_objtype_list($5, &n->for_all_tables, &n->for_all_sequences, yyscanner); n->options = $6; $$ = (Node *) n; } ``` Here we are assigning "n->pubobjects = (List *) $5". But later in the code this is not used anywhere for ALL TABLES/ ALL SEQUENCES publication. It is used for other publications (not ALL TABLES/ SEQUENCES) inside function "ObjectsInPublicationToOids" So are we required to assign "n->pubobjects" here? I have created a patch to remove this assignment. It passed "make check-world". Thanks, Shlok Kyal