On Wed, Jan 29, 2025 at 10:42 AM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > Dear Shubham, > > > I propose adding the --clean-publisher-objects option to the > > pg_createsubscriber utility. As discussed in [1], this feature ensures > > a clean and streamlined setup of logical replication by removing stale > > or unnecessary publications from the subscriber node. These > > publications, replicated during streaming replication, become > > redundant after converting to logical replication and serve no further > > purpose. This patch introduces the drop_all_publications() function, > > which efficiently fetches and drops all publications on the subscriber > > node within a single transaction. > > I think replication slot are also type of 'publisher-objects', but they are > not > removed for now: API-name may not be accurate. And... > > > Additionally, other related objects, such as subscriptions and > > replication slots, may also require cleanup. I plan to analyze this > > further and include them in subsequent patches. > > I'm not sure replication slots should be cleaned up. Apart from other items > like > publication/subscription, replication slots are not replicated when it is > created > on the primary instance. This means they are intentionally created by DBAs > and there > may not be no strong reasons to drop them after the conversion. > > Another question is the style of APIs. Do you plan to provide APIs like > 'cleanup-subscriber-objects' and 'cleanup-publisher-objects', or just one > 'cleanup-logical-replication-objects'? >
Thanks for the suggestions, I will keep them in mind while preparing
the 0002 patch for the same.
Currently, I have changed the API to '--cleanup-publisher-objects'.
> Regarding the patch:
>
> 1.
> ```
> + The <application>pg_createsubscriber</application> now supports the
> + <option>--clean-publisher-objects</option> to remove all publications
> on
> + the subscriber node before creating a new subscription.
> ```
>
> This description is not suitable for the documentation. Something like:
>
> Remove all publications on the subscriber node.
>
> 2.
> ```
> + /* Drop publications from the subscriber if requested */
> + drop_all_publications(dbinfo);
> ```
>
> This should be called when `opts.clean_publisher_objects` is true.
>
> 3.
> You said publications are dropped within a single transaction, but the
> patch does not do. Which is correct?
>
> 4.
> ```
> +# Set up node A as primary
> +my $node_a = PostgreSQL::Test::Cluster->new('node_a');
> +my $aconnstr = $node_a->connstr;
> +$node_a->init(allows_streaming => 'logical');
> +$node_a->append_conf('postgresql.conf', 'autovacuum = off');
> +$node_a->start;
> +
> +# Set up node B as standby linking to node A
> +$node_a->backup('backup_3');
> +my $node_b = PostgreSQL::Test::Cluster->new('node_b');
> +$node_b->init_from_backup($node_a, 'backup_3', has_streaming => 1);
> +$node_b->append_conf(
> + 'postgresql.conf', qq[
> + primary_conninfo = '$aconnstr'
> + hot_standby_feedback = on
> + max_logical_replication_workers = 5
> + ]);
> +$node_b->set_standby_mode();
> +$node_b->start;
> ```
>
Fixed the given comments. The attached patch contains the suggested changes.
Thanks and regards,
Shubham Khanna.
v2-0001-Support-for-dropping-all-publications-in-pg_creat.patch
Description: Binary data
