From: | "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com> |
---|---|
To: | Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: pg_createsubscriber: Fix incorrect handling of cleanup flags |
Date: | 2025-05-05 06:08:54 |
Message-ID: | CAKFQuwaqWa3tRNMaRYpa4jE7BVSMb1tPTeiwM9GE7781nKOPEQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, May 4, 2025 at 8:45 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)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.
From | Date | Subject | |
---|---|---|---|
Next Message | Amit Kapila | 2025-05-05 06:18:35 | Re: Add an option to skip loading missing publication to avoid logical replication failure |
Previous Message | DIPESH DHAMELIYA | 2025-05-05 05:48:42 | [PATCH] Allow parallelism for plpgsql return expression after commit 556f7b7 |