On Mon, Feb 24, 2025 at 10:57 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Tue, Feb 18, 2025 at 12:16 AM Shubham Khanna > <khannashubham1...@gmail.com> wrote: > > > > -static void check_publisher(const struct LogicalRepInfo *dbinfo); > -static char *setup_publisher(struct LogicalRepInfo *dbinfo); > +static void check_publisher(const struct LogicalRepInfo *dbinfo, bool > two_phase); > +static char *setup_publisher(struct LogicalRepInfo *dbinfo, bool two_phase); > static void check_subscriber(const struct LogicalRepInfo *dbinfo); > static void setup_subscriber(struct LogicalRepInfo *dbinfo, > - const char *consistent_lsn); > + const char *consistent_lsn, bool two_phase); > static void setup_recovery(const struct LogicalRepInfo *dbinfo, const > char *datadir, > const char *lsn); > static void drop_primary_replication_slot(struct LogicalRepInfo *dbinfo, > const char *slotname); > static void drop_failover_replication_slots(struct LogicalRepInfo *dbinfo); > static char *create_logical_replication_slot(PGconn *conn, > - struct LogicalRepInfo *dbinfo); > + struct LogicalRepInfo *dbinfo, > + bool two_phase); > static void drop_replication_slot(PGconn *conn, struct LogicalRepInfo > *dbinfo, > const char *slot_name); > static void pg_ctl_status(const char *pg_ctl_cmd, int rc); > @@ -98,7 +100,9 @@ static void wait_for_end_recovery(const char *conninfo, > const struct CreateSubscriberOptions *opt); > static void create_publication(PGconn *conn, struct LogicalRepInfo *dbinfo); > static void drop_publication(PGconn *conn, struct LogicalRepInfo *dbinfo); > -static void create_subscription(PGconn *conn, const struct > LogicalRepInfo *dbinfo); > +static void create_subscription(PGconn *conn, > + const struct LogicalRepInfo *dbinfo, > + bool two_phase); > > Specifying two_phase as a separate parameter in so many APIs looks > odd. Wouldn't it be better to make it part of LogicalRepInfo as we do > with other parameters of CreateSubscriberOptions? >
I agree that passing two_phase as a separate parameter across multiple function calls adds redundancy. It would be more consistent and maintainable to include two_phase as part of LogicalRepInfo, similar to how other options from CreateSubscriberOptions are handled. I have created a new structure LogicalRepInfos to include a bool two_phase field. This way, all functions that require two_phase can access it directly from the LogicalRepInfos structure, reducing unnecessary argument passing. The attached patch contains the suggested changes. Thanks and regards, Shubham Khanna.
v16-0001-Add-support-for-two-phase-commit-in-pg_createsub.patch
Description: Binary data
