On Fri, Oct 17, 2025 at 1:35 AM shveta malik <[email protected]> wrote: > > On Fri, Oct 17, 2025 at 10:01 AM Amit Kapila <[email protected]> wrote: > > > > On Thu, Oct 16, 2025 at 4:53 PM Zhijie Hou (Fujitsu) > > <[email protected]> wrote: > > > > > > Regarding whether we can avoid creating slot/origin for seq-only > > > publication. > > > I think the main challenge lies in ensuring the apply worker operates > > > smoothly > > > without a replication slot. Currently, the apply worker uses the > > > START_REPLICATION command with a replication slot to acquire the slot on > > > the > > > publisher. To bypass this, it's essential to skip starting the > > > replication and > > > specifically, avoid entering the LogicalRepApplyLoop(). > > > > > > To address this, I thought to implement a separate loop dedicated to > > > sequence-only subscriptions. Within this loop, the apply worker would > > > only call > > > functions like ProcessSyncingSequencesForApply() to manage sequence > > > synchronization while periodically checking for any new tables added to > > > the > > > subscription. If new tables are detected, the apply worker would exit > > > this loop > > > and enter the LogicalRepApplyLoop(). > > > > > > I chose not to consider allowing the START_REPLICATION command to operate > > > without a logical slot, as it seems like an unconventional approach > > > requiring > > > modifications in walsender and to skip logical decoding and related > > > processes. > > > > > > Another consideration is whether to address scenarios where tables are > > > subsequently removed from the subscription, given that slots and origins > > > would > > > already have been created in such cases. > > > > > > Since it might introduce addition complexity to the patches, and > > > considering > > > that we already allow slot/origin to be created for empty subscription, > > > it might > > > also be acceptable to allow it to be created for sequence-only > > > subscription. So, > > > I chose to add some comments to explain the reason for it in latest > > > version. > > > > > > Origin case might be slightly easier to handle, but it could also require > > > some > > > amount of implementations. Since origin is less harmful than a > > > replication slot > > > and maintaining it does not have noticeable overhead, it seems OK to me to > > > retain the current behaviour and add some comments in the patch to > > > clarify the > > > same. > > > > > > > I agree that avoiding to create a slot/origin for sequence-only > > subscription is not worth the additional complexity at other places, > > especially when we do create them for empty subscriptions. > > +1. > > While testeing 001 patch alone, I found that for sequence-only > subscription, we get error in tablesync worker : > ERROR: relation "public.seq1" type mismatch: source "table", target > "sequence" > > This error comes because during copy_table(), > logicalrep_relmap_update() does not update relkind and thus later > CheckSubscriptionRelkind() ends up giving the above error.
I faced the same error while reviewing the 0001 patch. I think if we're going to push these patches separately the 0001 patch should have at least minimal regression tests. Otherwise, I'm concerned that buildfarm animals won't complain but we could end up blocking other logical replication developments. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
