Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pg_createsubscriber: Fix incorrect handling of cleanup flags
Date: 2026-05-22 05:27:54
Message-ID: CAHGQGwH6MtZ9dN-zeF7SjqzCmP-e4DKERYWp9zYEofUnqb_-Kg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, May 12, 2026 at 7:56 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
> Let me first briefly explain why this patch still holds:
> The made_publication and made_replslot are publisher-side cleanup
> flags. They are checked only in cleanup_objects_atexit(), which
> executes at most once and connects to the publisher to remove objects
> created by the tool. They have no inherent relationship to the
> subscriber.
> That said, check_and_drop_publications() always operates on the
> subscriber, so resetting made_publication because of a failure on the
> subscriber side, when that same flag is later used for publisher-side
> cleanup, does not seem correct to me as it could incorrectly skip the
> required cleanup. A failure on one server should not affect cleanup
> decisions for another server. The same is true for the made_replslot
> flag in respective code.
>
> After looking further, I noticed that since a recent commit
> 85ddcc2[3], check_and_drop_publications() also reads made_publication
> to decide whether the subscriber-side inherited publication should be
> dropped or preserved. In that sense, the dual usage of the flag looks
> valid and continues to work correctly with my patch.
>
> Also, after checking on pg_head, I don’t think there is a possibility
> of a double-drop on the subscriber side. Either all publications are
> dropped in the if (drop_all_pubs) block when "--remove=publications"
> is specified, or by default the else block drops only dbinfo->pubname
> (the FOR ALL TABLES publication). The if (made_publication) condition
> simply guards against dropping a pre-existing publication. I don’t see
> an issue there, but please let me know if I’m missing something.

Thanks for the analysis of this issue!

Barring any objections, I will commit the patch and backpatch it to v17.

Regards,

--
Fujii Masao

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-05-22 05:30:12 Re: Support EXCEPT for TABLES IN SCHEMA publications
Previous Message Nisha Moond 2026-05-22 04:51:21 Re: Proposal: Conflict log history table for Logical Replication