| From: | Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com> |
|---|---|
| To: | vignesh C <vignesh21(at)gmail(dot)com> |
| Cc: | shveta malik <shveta(dot)malik(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Shinya Kato <shinya11(dot)kato(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Euler Taveira <euler(at)eulerto(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
| Subject: | Re: Logical Replication of sequences |
| Date: | 2025-11-10 09:03:54 |
| Message-ID: | CANhcyEWbjkFvk3mSy5LFs9+0z4K1gDwQeFj7GUjOe+L4vxs4AA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Fri, 7 Nov 2025 at 20:18, vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Fri, 7 Nov 2025 at 14:54, shveta malik <shveta(dot)malik(at)gmail(dot)com> wrote:
> >
> > On Fri, Nov 7, 2025 at 10:58 AM vignesh C <vignesh21(at)gmail(dot)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
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Remove-unused-n-pubobjects-assignment-for-CREATE-.patch | application/octet-stream | 1.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Chao Li | 2025-11-10 09:06:07 | Re: [PATCH] Add hints for invalid binary encoding names in encode/decode functions |
| Previous Message | Jakub Wartak | 2025-11-10 08:59:58 | Re: postmaster uses more CPU in 18 beta1 with io_method=io_uring |