Re: Logical Replication of sequences

On Mon, 5 Aug 2024 at 17:28, Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Aug 5, 2024 at 2:36 PM vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Fri, 2 Aug 2024 at 14:24, shveta malik <shveta.malik@gmail.com> wrote:
> > >
> > > On Thu, Aug 1, 2024 at 9:26 AM shveta malik <shveta.malik@gmail.com> wrote:
> > > >
> > > > On Mon, Jul 29, 2024 at 4:17 PM vignesh C <vignesh21@gmail.com> wrote:
> > > > >
> > > > > Thanks for reporting this, these issues are fixed in the attached
> > > > > v20240730_2 version patch.
> > > > >
> > >
> > > I was reviewing the design of patch003, and I have a query. Do we need
> > > to even start an apply worker and create replication slot when
> > > subscription created is for 'sequences only'? IIUC, currently logical
> > > replication apply worker is the one launching sequence-sync worker
> > > whenever needed. I think it should be the launcher doing this job and
> > > thus apply worker may even not be needed for current functionality of
> > > sequence sync?
> >
>
> But that would lead to maintaining all sequence-sync of each
> subscription by launcher. Say there are 100 sequences per subscription
> and some of them from each subscription are failing due to some
> reasons then the launcher will be responsible for ensuring all the
> sequences are synced. I think it would be better to handle
> per-subscription work by the apply worker.
>
> >
> > Going forward when we implement incremental sync of
> > > sequences, then we may have apply worker started but now it is not
> > > needed.
> >
> > I believe the current method of having the apply worker initiate the
> > sequence sync worker is advantageous for several reasons:
> > a) Reduces Launcher Load: This approach prevents overloading the
> > launcher, which must handle various other subscription requests.
> > b) Facilitates Incremental Sync: It provides a more straightforward
> > path to extend support for incremental sequence synchronization.
> > c) Reuses Existing Code: It leverages the existing tablesync worker
> > code for starting the tablesync process, avoiding the need to
> > duplicate code in the launcher.
> > d) Simplified Code Maintenance: Centralizing sequence synchronization
> > logic within the apply worker can simplify code maintenance and
> > updates, as changes will only need to be made in one place rather than
> > across multiple components.
> > e) Better Monitoring and Debugging: With sequence synchronization
> > being handled by the apply worker, you can more effectively monitor
> > and debug synchronization processes since all related operations are
> > managed by a single component.
> >
> > Also, I noticed that even when a publication has no tables, we create
> > replication slot and start apply worker.
> >
>
> As far as I understand slots and origins are primarily required for
> incremental sync. Would it be used only for sequence-sync cases? If
> not then we can avoid creating those. I agree that it would add some
> complexity to the code with sequence-specific checks, so we can create
> a top-up patch for this if required and evaluate its complexity versus
> the benefit it produces.

I have added a XXX todo comments in the v20240807 version patch
attached at [1]. I will handle this as a separate patch once the
current patch is stable.
[1] - https://www.postgresql.org/message-id/CALDaNm01Z6Oo9osGMFTOoyTR1kVoyh1rEvZ%2B6uJn-ZymV%3D0dbQ%40mail.gmail.com

Regards,
Vignesh