Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
Cc: Shubham Khanna <khannashubham1197(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, "Hayato Kuroda (Fujitsu)" <kuroda(dot)hayato(at)fujitsu(dot)com>, Shlok Kyal <shlok(dot)kyal(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Adding a '--clean-publisher-objects' option to 'pg_createsubscriber' utility.
Date: 2025-03-11 07:20:04
Message-ID: CAHut+Pt0-KuSKqTuYaRYNrd634gR4ddnrpPsCnaSmEnHVZEucQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Mar 11, 2025 at 11:32 AM David G. Johnston
<david(dot)g(dot)johnston(at)gmail(dot)com> wrote:
>
> On Mon, Mar 10, 2025 at 5:00 PM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
>>
>> Hi Shubham.
>>
>> Some review comments for patch v16-0001.
>>
>> ======
>> doc/src/sgml/ref/pg_createsubscriber.sgml
>>
>> 1.
>> + <term><option>-c</option></term>
>> + <term><option>--drop-all-publications</option></term>
>>
>> Is 'c' the best switch choice letter for this option? It doesn't seem
>> intuitive, but unfortunately, I don't have any better ideas since d/D
>> and p/P are already being used.
>
>
> Agreed. Better to just not assign a short name to this.
>
> The description for the sgml docs needs to frame up this option's purpose.
>
> How exactly does one go about backing up a publication? You discuss the topic in the commit message but that definitely seems user-facing.
>
> If we aren't expecting lots of publications maybe name them individually instead of logging "all publications"? Possibly one info line each but even just comma separated would work.
>
> The name of this is shock-inducing. Admittedly, after pondering things, it is fairly obvious that only the target is going to be affected, but there is a source database involved here as well. It is also unclear on whether it would happen before or after, which is less problematic since it would only impact failure modes anyway - when all is said and done with this specified upon restart following the pg_resetwal the server will have no publications, right?
>
> Maybe: --drop-target-publications-first ?
>
> If we do want a letter either "X" or "Z" probably works for an English-speaking audience probably. X is how one denotes removing something; and Z is a mnemonic for "Zap" which is a synonym for "Drop". "R" for "Remove".
>
> Can you briefly recap how this is different than the automatic behavior described in the existing Step 6?
> "Drop publications on the target server that were replicated because they were created before the replication start location. It has no use on the subscriber."
>
> -R --remove-target-publications; I fine with answering the timing question to the description.
>

Hi David.

This feature is all about removing some objects that were replicated
during the streaming replication before pg_createsubscriber was run,
but that are no longer needed/wanted afterwards.

Although the current thread/patch is just for removing existing
publications from the target server, in the future there might be some
enhancements to remove other kinds of unwanted objects that were
replicated to the target server by streaming replication -- e.g.
remove subscriptions, or remove replication slots, or whatever.

Unfortunately, we are spinning in circles a bit trying to come up with
a good way to represent the option needed for this, while at the same
time trying to be future-proof. I see 3 choices...

======

Choice 1. Generic option

Implement a single boolean option to remove everything.

In this case, the option would need to have some generic name like
"--remove-target-existing-object". For now (current patch), it would
be implemented to remove only existing publications. In the future, it
could remove more things. But, there isn't much flexibility: IIUC this
option can only remove either all or none.

~

Choice 2. Different options for removing different things (the current
patch is like this)

"--remove-target-publications" is what this patch is doing.

In future, if we want to remove more things then we would need to add
more options like "--remove-target-subscriptions",
"--remove-target-replication-slots" etc.

~

Choice 3. Implement some option that has an argument saying what to delete

Implement an option that takes some argument saying what objects to remove.

Here, the current patch would be something like:
"--remove-target-objects=publications". In future, this option could
be enhanced to accept other values like
"--remove-target-objects=publications,subscriptions" etc.

~~~

Do you have any thoughts on what kind of option is best here?

======
Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2025-03-11 07:21:40 Re: SQL Property Graph Queries (SQL/PGQ)
Previous Message Michael Paquier 2025-03-11 07:06:14 Re: Tidy recent code bloat in pg_creatersubscriber::cleanup_objects_atexit