Re: Adding a '--two-phase' option to 'pg_createsubscriber' utility.

On Thu, Dec 12, 2024 at 9:34 AM vignesh C <vignesh21@gmail.com> wrote:
>
> On Thu, 12 Dec 2024 at 08:14, Hayato Kuroda (Fujitsu)
> <kuroda.hayato@fujitsu.com> wrote:
> >
> > Dear Shubham,
> >
> > > Thank you for pointing this out and for suggesting the changes. I
> > > agree with your approach.
> > > Also, I found a mistake in getopt_long and fixed it in this version of
> > > the patch.
> > > The attached patch contains the suggested changes.
> >
> > Thanks for updating the patch. I think the patch looks mostly OK.
> >
> > Regarding the test code - I think we should directly refer the pg_subscription catalog,
> > and confirm that subtwophase is 'p'. IIUC, we can wait until
> > all subscriptions. Subtwophasestate are 'e', by using poll_query_until() and [1].
> >
> > [1]: SELECT count(1) = 0 FROM pg_subscription WHERE subtwophasestate NOT IN ('e');
>
> Yes, that approach is better. Also this is not required after checking
> using pg_subscription:
> +# Prepare a transaction on the publisher
> +$node_p->safe_psql(
> + $db1, qq[
> +        BEGIN;
> +        INSERT INTO tbl1 SELECT generate_series(1, 10);
> +        PREPARE TRANSACTION 'test_prepare';
> +]);
> +
>  # Start subscriber
>  $node_s->start;
>
> +# Verify that the prepared transaction is replicated to the subscriber
> +my $count_prepared_s =
> +  $node_s->safe_psql($db1, "SELECT count(*) FROM pg_prepared_xacts;");
> +
> +is($count_prepared_s, qq(1), 'Prepared transaction replicated to subscriber');
>

I have fixed the given comment. The v5 version patch attached at [1]
has the changes for the same.
[1] - https://www.postgresql.org/message-id/CAHv8Rj%2Bhd2MTNRs4AsK6%3DhRqvV6JC9g2tTAJwGjrNfXg6vhD8g%40mail.gmail.com

Thanks and Regards,
Shubham Khanna.