On Tue, May 6, 2025 at 11:11 AM Nisha Moond <[email protected]> wrote: > > On Mon, May 5, 2025 at 11:39 AM David G. Johnston > <[email protected]> wrote: > > > > On Sun, May 4, 2025 at 8:45 PM Nisha Moond <[email protected]> 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
v3-0001-Fix-incorrect-cleanup-flag-handling-in-pg_creates.patch
Description: Binary data
