| 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
| 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 |