pg_createsubscriber: Fix cleanup of publisher-side objects after errors

When pg_createsubscriber fails after creating logical replication
objects, it should remove the publication and replication slot that
it created on the publisher.

Previously, if dropping subscriber-side objects failed,
pg_createsubscriber reset its internal cleanup state too early. As a
result, the exit-time cleanup could skip removing the publication or
replication slot on the publisher.

This could leave pg_createsubscriber-created objects behind on
the publisher after a failed run. That can make a retry harder,
because the leftover publication or replication slot may need to be
removed manually before running pg_createsubscriber again.
In the case of a replication slot, leaving it behind can also retain
WAL files longer than expected.

The cause of this issue was that the flags made_publication and
made_replslot tracking whether pg_createsubscriber created
a publication or replication slot on the primary were incorrectly
reset to false when failures occurred while dropping objects
on the subscriber.

This commit fixes the issue by preventing those cleanup flags from
being reset even when failures occurred while dropping objects
on the subscriber, ensuring proper cleanup of primary objects
before exit on failure.

Backpatch to v17, where pg_createsubscriber was added.

Author: Nisha Moond <nisha.moond412@gmail.com>
Reviewed-by: David G. Johnston <david.g.johnston@gmail.com>
Reviewed-by: Fujii Masao <masao.fujii@gmail.com>
Reviewed-by: Peter Smith <smithpb2250@gmail.com>
Discussion: https://postgr.es/m/CABdArM5V9QKK1PkLY9dpgAcZa3kUp84-wPqPovxvdLOri4=69w@mail.gmail.com
Backpatch-through: 17

diff --git a/src/bin/pg_basebackup/pg_createsubscriber.c b/src/bin/pg_basebackup/pg_createsubscriber.c
index 81d3c78b805acdaae6b59f7ce5f239006fb3982a..cb16a60800280503a983476a518580299068834c 100644 (file)
--- a/src/bin/pg_basebackup/pg_createsubscriber.c
+++ b/src/bin/pg_basebackup/pg_createsubscriber.c
@@ -139,7 +139,7 @@ static void wait_for_end_recovery(const char *conninfo,
 static void create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo);
 static bool find_publication(PGconn *conn, const char *pubname, const char *dbname);
 static void drop_publication(PGconn *conn, const char *pubname,
-                                                        const char *dbname, bool *made_publication);
+                                                        const char *dbname);
 static void check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo);
 static void create_subscription(PGconn *conn, const struct LogicalRepInfo *dbinfo);
 static void set_replication_progress(PGconn *conn, const struct LogicalRepInfo *dbinfo,
@@ -245,8 +245,7 @@ cleanup_objects_atexit(void)
                        if (conn != NULL)
                        {
                                if (dbinfo->made_publication)
-                                       drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
-                                                                        &dbinfo->made_publication);
+                                       drop_publication(conn, dbinfo->pubname, dbinfo->dbname);
                                if (dbinfo->made_replslot)
                                        drop_replication_slot(conn, dbinfo, dbinfo->replslotname);
                                disconnect_database(conn, false);
@@ -1629,7 +1628,6 @@ drop_replication_slot(PGconn *conn, struct LogicalRepInfo *dbinfo,
                {
                        pg_log_error("could not drop replication slot \"%s\" in database \"%s\": %s",
                                                 slot_name, dbinfo->dbname, PQresultErrorMessage(res));
-                       dbinfo->made_replslot = false;  /* don't try again. */
                }
 
                PQclear(res);
@@ -1871,8 +1869,7 @@ create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo)
  * Drop the specified publication in the given database.
  */
 static void
-drop_publication(PGconn *conn, const char *pubname, const char *dbname,
-                                bool *made_publication)
+drop_publication(PGconn *conn, const char *pubname, const char *dbname)
 {
        PQExpBuffer str = createPQExpBuffer();
        PGresult   *res;
@@ -1902,7 +1899,6 @@ drop_publication(PGconn *conn, const char *pubname, const char *dbname,
                {
                        pg_log_error("could not drop publication \"%s\" in database \"%s\": %s",
                                                 pubname, dbname, PQresultErrorMessage(res));
-                       *made_publication = false;      /* don't try again. */
 
                        /*
                         * Don't disconnect and exit here. This routine is used by primary
@@ -1951,8 +1947,7 @@ check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo)
 
                /* Drop each publication */
                for (int i = 0; i < PQntuples(res); i++)
-                       drop_publication(conn, PQgetvalue(res, i, 0), dbinfo->dbname,
-                                                        &dbinfo->made_publication);
+                       drop_publication(conn, PQgetvalue(res, i, 0), dbinfo->dbname);
 
                PQclear(res);
        }
@@ -1961,8 +1956,7 @@ check_and_drop_publications(PGconn *conn, struct LogicalRepInfo *dbinfo)
                /* Drop publication only if it was created by this tool */
                if (dbinfo->made_publication)
                {
-                       drop_publication(conn, dbinfo->pubname, dbinfo->dbname,
-                                                        &dbinfo->made_publication);
+                       drop_publication(conn, dbinfo->pubname, dbinfo->dbname);
                }
                else
                {