Horton

Thread

Commits

  1. pg_createsubscriber: Fix cleanup of publisher-side objects after errors

  2. pg_createsubscriber: Introduce module-specific logging functions.

  3. Support existing publications in pg_createsubscriber.

  4. pg_createsubscriber: Add -R publications option.

  5. pg_createsubscriber: creates a new logical replica from a standby server

  1. pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2025-05-05T03:45:35Z

    Hi Hackers,

In pg_createsubscriber, the flags 'made_publication' and
'made_replslot' are used to track whether the tool itself created a
publication or replication slot on the primary (publisher) node. If a
failure happens during the process, these flags help the tool decide
whether it should clean up those objects using
cleanup_objects_atexit().
However, there are cases where these flags are wrongly set to false
due to failures on the subscriber side, which causes the tool to skip
cleanup of some objects on the primary even when it should not.

Example: for made_publication
  In drop_publication(), if dropping a publication on the subscriber
fails (either a replicated publication or an existing one being
removed with --remove=publications), the made_publication flag is
wrongly set to false.
The process continues without exiting, but if a later step fails,
cleanup_objects_atexit() will see made_publication = false and skip
dropping the publication on the primary, even though it was created
earlier by the tool. This leads to orphaned publication.

Example: for made_replslot
  A similar issue exists for replication slots. In
drop_replication_slot(), if dropping a physical replication slot on
the primary, or a failover-synced slot on the subscriber, fails — the
made_replslot flag is set to false.
Again, if the process fails later, cleanup_objects_atexit() will
incorrectly skip dropping the logical replication slot created earlier
on the primary, leaving it behind.

Solution:
  The fix ensures that failures in dropping subscriber-side or
non-internal objects should not reset made_publication or
made_replslot.
These flags should only be reset if dropping the internally created
objects on the primary fails. That way, cleanup_objects_atexit() can
still correctly clean up what the tool created if something else goes
wrong later in the process.

Attached is the patch implementing the above proposed solution.
Reviews and feedback are most welcome.

--
Thanks,
Nisha

  2. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    David G. Johnston <david.g.johnston@gmail.com> — 2025-05-05T06:08:54Z

    On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:

> Attached is the patch implementing the above proposed solution.
> Reviews and feedback are most welcome.
>

I feel like this is just papering over the issue - which is that these two
drop functions are being used for multiple differently named
publications/slots yet take no care to ensure they only change
made_publication and made_repslot if the name of the object being passed in
matches the name of the object those two booleans are specifically tracking
(the application created objects on the publisher).

Make it so they are only changed to false if the name matches the one the
program created and the connection is the primary connection.  That targets
the real issue and avoids using a branching boolean parameter.

It seems really odd to say: if (in_cleanup) "don't try again" - since by
definition this is the last thing we are doing before we exit.  So really
what this patch can do more simply is just remove the
dbinfo->made_replslot=false and *made_publication=false lines altogether -
which might be a valid option.

I'm partial to the latter really, I don't think the error message output
for retrying a drop that may have previously failed would be an issue.

David J.

  3. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2025-05-06T05:41:00Z

    On Mon, May 5, 2025 at 11:39 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:
>
> On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>>
>> Attached is the patch implementing the above proposed solution.
>> Reviews and feedback are most welcome.
>
>
> I feel like this is just papering over the issue - which is that these two drop functions are being used for multiple differently named publications/slots yet take no care to ensure they only change made_publication and made_repslot if the name of the object being passed in matches the name of the object those two booleans are specifically tracking (the application created objects on the publisher).
>
> Make it so they are only changed to false if the name matches the one the program created and the connection is the primary connection.  That targets the real issue and avoids using a branching boolean parameter.
>
> It seems really odd to say: if (in_cleanup) "don't try again" - since by definition this is the last thing we are doing before we exit.  So really what this patch can do more simply is just remove the dbinfo->made_replslot=false and *made_publication=false lines altogether - which might be a valid option.
>

+1 to removing the dbinfo->made_replslot=false and
*made_publication=false lines. In my tests, I attempted to force
multiple failures, but couldn’t find any case where
cleanup_objects_atexit() would recurse or cause repeated cleanup if
these flags remain set to true.

> I'm partial to the latter really, I don't think the error message output for retrying a drop that may have previously failed would be an issue.
>

As of now, we don’t attempt to drop the same object more than once, so
the latter approach does seem reasonable to me. That said, I’m unsure
why the flags were being reset here in the first place.

Please find the updated patch which removes the false setting of these
flags during drop. If there’s a case I’ve overlooked where this might
be problematic, we can certainly go for your first suggestion to match
the names.

--
Thanks,
Nisha

  4. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2026-04-01T08:14:00Z

    On Tue, May 6, 2025 at 11:11 AM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Mon, May 5, 2025 at 11:39 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> >
> > On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >>
> >> Attached is the patch implementing the above proposed solution.
> >> Reviews and feedback are most welcome.
> >
> >
> > I feel like this is just papering over the issue - which is that these two drop functions are being used for multiple differently named publications/slots yet take no care to ensure they only change made_publication and made_repslot if the name of the object being passed in matches the name of the object those two booleans are specifically tracking (the application created objects on the publisher).
> >
> > Make it so they are only changed to false if the name matches the one the program created and the connection is the primary connection.  That targets the real issue and avoids using a branching boolean parameter.
> >
> > It seems really odd to say: if (in_cleanup) "don't try again" - since by definition this is the last thing we are doing before we exit.  So really what this patch can do more simply is just remove the dbinfo->made_replslot=false and *made_publication=false lines altogether - which might be a valid option.
> >
>
> +1 to removing the dbinfo->made_replslot=false and
> *made_publication=false lines. In my tests, I attempted to force
> multiple failures, but couldn’t find any case where
> cleanup_objects_atexit() would recurse or cause repeated cleanup if
> these flags remain set to true.
>
> > I'm partial to the latter really, I don't think the error message output for retrying a drop that may have previously failed would be an issue.
> >
>
> As of now, we don’t attempt to drop the same object more than once, so
> the latter approach does seem reasonable to me. That said, I’m unsure
> why the flags were being reset here in the first place.
>
> Please find the updated patch which removes the false setting of these
> flags during drop. If there’s a case I’ve overlooked where this might
> be problematic, we can certainly go for your first suggestion to match
> the names.
>

The patch needed rebasing after recent changes in pg_createsubscriber
under commit d6628a5.
Please find the rebased patch (v3) attached.

--
Thanks,
Nisha

  5. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2026-05-10T04:30:01Z

    On Wed, Apr 1, 2026 at 1:44 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> The patch needed rebasing after recent changes in pg_createsubscriber
> under commit d6628a5.
> Please find the rebased patch (v3) attached.
>

Needed a rebase after recent commits. Attached the rebased v4 patch.

--
Thanks,
Nisha

  6. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Fujii Masao <masao.fujii@gmail.com> — 2026-05-11T01:22:55Z

    On Sun, May 10, 2026 at 1:30 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Wed, Apr 1, 2026 at 1:44 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > The patch needed rebasing after recent changes in pg_createsubscriber
> > under commit d6628a5.
> > Please find the rebased patch (v3) attached.
> >
>
> Needed a rebase after recent commits. Attached the rebased v4 patch.

Thanks for updating the patch! It looks good to me.

We can remove the third argument, made_publication, from drop_publication()
since it is no longer used?

Regards,

-- 
Fujii Masao

  7. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2026-05-11T15:55:12Z

    On Mon, May 11, 2026 at 6:53 AM Fujii Masao <masao.fujii@gmail.com> wrote:
>
> On Sun, May 10, 2026 at 1:30 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Wed, Apr 1, 2026 at 1:44 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > >
> > > The patch needed rebasing after recent changes in pg_createsubscriber
> > > under commit d6628a5.
> > > Please find the rebased patch (v3) attached.
> > >
> >
> > Needed a rebase after recent commits. Attached the rebased v4 patch.
>
> Thanks for updating the patch! It looks good to me.
>
> We can remove the third argument, made_publication, from drop_publication()
> since it is no longer used?
>

+1, attached the updated patch.

--
Thanks,
Nisha

  8. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Peter Smith <smithpb2250@gmail.com> — 2026-05-12T02:02:36Z

    On Tue, May 6, 2025 at 3:41 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
>
> On Mon, May 5, 2025 at 11:39 AM David G. Johnston
> <david.g.johnston@gmail.com> wrote:
> >
> > On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >>
> >> Attached is the patch implementing the above proposed solution.
> >> Reviews and feedback are most welcome.
> >
> >
> > I feel like this is just papering over the issue - which is that these two drop functions are being used for multiple differently named publications/slots yet take no care to ensure they only change made_publication and made_repslot if the name of the object being passed in matches the name of the object those two booleans are specifically tracking (the application created objects on the publisher).
> >
> > Make it so they are only changed to false if the name matches the one the program created and the connection is the primary connection.  That targets the real issue and avoids using a branching boolean parameter.
> >
> > It seems really odd to say: if (in_cleanup) "don't try again" - since by definition this is the last thing we are doing before we exit.  So really what this patch can do more simply is just remove the dbinfo->made_replslot=false and *made_publication=false lines altogether - which might be a valid option.
> >
>
> +1 to removing the dbinfo->made_replslot=false and
> *made_publication=false lines. In my tests, I attempted to force
> multiple failures, but couldn’t find any case where
> cleanup_objects_atexit() would recurse or cause repeated cleanup if
> these flags remain set to true.
>
> > I'm partial to the latter really, I don't think the error message output for retrying a drop that may have previously failed would be an issue.
> >
>
> As of now, we don’t attempt to drop the same object more than once, so
> the latter approach does seem reasonable to me. That said, I’m unsure
> why the flags were being reset here in the first place.
>

Hi Nisha,

From what I can tell, this flag resetting might have originated from
old review comments of mine [1 #5b] and also [2 #6].

Apparently, it wasn't so much about preventing multiple errors for
fdrops that had "previously failed"... it was more intended to prevent
attempting to delete the same publication 2x (1st successfully, then
2nd time failing because it was already gone)

Anyway, the code has probably changed lots since those review
comments; I have no idea if my suspected double-delete at that time is
still possible or not. Those old review comments might give you some
clues on how to reproduce now that you have removed the flag
resetting.

======
[1] https://www.postgresql.org/message-id/CAHut%2BPt_4f-%3Dh71qmfWhiFcqTcfqWQr1POnFdesZK1-fVOCaUA%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAHut%2BPtRZbuF_vAva6azC%2B6WVPyaoqT1fCeLNP0r5gwPEsfcwg%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

  9. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2026-05-12T10:56:29Z

    On Tue, May 12, 2026 at 7:33 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> On Tue, May 6, 2025 at 3:41 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> >
> > On Mon, May 5, 2025 at 11:39 AM David G. Johnston
> > <david.g.johnston@gmail.com> wrote:
> > >
> > > On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > >>
> > >> Attached is the patch implementing the above proposed solution.
> > >> Reviews and feedback are most welcome.
> > >
> > >
> > > I feel like this is just papering over the issue - which is that these two drop functions are being used for multiple differently named publications/slots yet take no care to ensure they only change made_publication and made_repslot if the name of the object being passed in matches the name of the object those two booleans are specifically tracking (the application created objects on the publisher).
> > >
> > > Make it so they are only changed to false if the name matches the one the program created and the connection is the primary connection.  That targets the real issue and avoids using a branching boolean parameter.
> > >
> > > It seems really odd to say: if (in_cleanup) "don't try again" - since by definition this is the last thing we are doing before we exit.  So really what this patch can do more simply is just remove the dbinfo->made_replslot=false and *made_publication=false lines altogether - which might be a valid option.
> > >
> >
> > +1 to removing the dbinfo->made_replslot=false and
> > *made_publication=false lines. In my tests, I attempted to force
> > multiple failures, but couldn’t find any case where
> > cleanup_objects_atexit() would recurse or cause repeated cleanup if
> > these flags remain set to true.
> >
> > > I'm partial to the latter really, I don't think the error message output for retrying a drop that may have previously failed would be an issue.
> > >
> >
> > As of now, we don’t attempt to drop the same object more than once, so
> > the latter approach does seem reasonable to me. That said, I’m unsure
> > why the flags were being reset here in the first place.
> >
>
> Hi Nisha,
>

Hi Peter, thank you for looking into it.

> From what I can tell, this flag resetting might have originated from
> old review comments of mine [1 #5b] and also [2 #6].
>

These flag resettings were already present since commit d44032d[1],
which introduced pg_createsubscriber. The changes discussed in your
comments for commit e5aeed4[2] did not modify their behavior.

> Apparently, it wasn't so much about preventing multiple errors for
> fdrops that had "previously failed"... it was more intended to prevent
> attempting to delete the same publication 2x (1st successfully, then
> 2nd time failing because it was already gone)
>
> Anyway, the code has probably changed lots since those review
> comments; I have no idea if my suspected double-delete at that time is
> still possible or not. Those old review comments might give you some
> clues on how to reproduce now that you have removed the flag
> resetting.
>

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.

[1] https://github.com/postgres/postgres/commit/d44032d
[2] https://github.com/postgres/postgres/commit/e5aeed4
[3] https://github.com/postgres/postgres/commit/85ddcc2

--
Thanks,
Nisha

  10. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Fujii Masao <masao.fujii@gmail.com> — 2026-05-22T05:27:54Z

    On Tue, May 12, 2026 at 7:56 PM Nisha Moond <nisha.moond412@gmail.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

  11. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Fujii Masao <masao.fujii@gmail.com> — 2026-05-27T01:37:30Z

    On Fri, May 22, 2026 at 2:27 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> Barring any objections, I will commit the patch and backpatch it to v17.

I've pushed the patch. Thanks!

Regards,

-- 
Fujii Masao

  12. Re: pg_createsubscriber: Fix incorrect handling of cleanup flags

    Nisha Moond <nisha.moond412@gmail.com> — 2026-05-28T03:12:03Z

    On Wed, 27 May, 2026, 7:07 am Fujii Masao, <masao.fujii@gmail.com> wrote:
>
> On Fri, May 22, 2026 at 2:27 PM Fujii Masao <masao.fujii@gmail.com> wrote:
> > Barring any objections, I will commit the patch and backpatch it to v17.
>
> I've pushed the patch. Thanks!
>

Thank you Fujii-san for pushing the fix.

--
Regards,
Nisha