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

On Wed, Jan 15, 2025 at 9:24 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
>
>
>
> On Wed, Jan 15, 2025 at 5:33 PM Shubham Khanna <khannashubham1197(at)gmail(dot)com> wrote:
> > Previously, the warning was necessary because the 'two-phase' option
> > was not available, and users needed to be informed about the default
> > behavior regarding 'two-phase' commit. However, with the recent
> > addition of the 'two-phase' option, users can now directly configure
> > this behavior during the setup process.
> > Given this enhancement, the warning message is no longer relevant and
> > should be removed from the documentation to reflect the latest changes
> > accurately. Updating the documentation will help ensure that it aligns
> > with the current functionality and avoids any potential confusion for
> > users.
>
> Hi Shubham,
>
> Even though the documentation is updated, the actual code still gives a warning, when you try and create pg_createsubscriber with the --enable-two-phase option:
>
> pg_createsubscriber: warning: two_phase option will not be enabled for replication slots
> pg_createsubscriber: detail: Subscriptions will be created with the two_phase option disabled. Prepared transactions will be replicated at COMMIT PREPARED.
>
> This is coming from code in check_publisher()
>
> if (max_prepared_transactions != 0)
> {
> pg_log_warning("two_phase option will not be enabled for replication slots");
> pg_log_warning_detail("Subscriptions will be created with the two_phase option disabled. "
> "Prepared transactions will be replicated at COMMIT PREPARED.");
> }
>
> I think this code needs to be updated as well.
>

Hi Shubham.

I'm not so sure about the documentation change of v10.

Sure, we can remove the warnings in the docs and just assume/expect
the user to be aware that there is a new '--enable-two-phase' option
that they should be using. That is what v10 is now doing.

OTOH, if the user (accidentally?) omits the new '--enable-two-phase'
switch then all this two-phase documentation still seems relevant.

~

In other words, we could've left all this documented information
intact, but just qualify the paragraph. For example:

CURRENTLY (v9)
pg_createsubscriber sets up logical replication with two-phase commit
disabled. This means that any prepared transactions will be replicated
at the time of COMMIT PREPARED, without advance preparation. Once
setup is complete, you can manually drop and re-create the
subscription(s) with the two_phase option enabled.

SUGGESTION
Unless the '--enable-two-phase' switch is specified,
pg_createsubscriber sets up ...

~

Similarly, to help (accident prone?) users we could leave this code
warning [1] intact, but just change the condition slightly, and add a
helpful hint on how to avoid the problem.

CURRENTLY (v10)
if (max_prepared_transactions != 0)
{
pg_log_warning("two_phase option will not be enabled for
replication slots");
pg_log_warning_detail("Subscriptions will be created with the
two_phase option disabled. "
"Prepared transactions will be
replicated at COMMIT PREPARED.");
}

SUGGESTION
if (max_prepared_transactions != 0 && !opt->two_phase)
{
pg_log_warning("two_phase option will not be enabled for
replication slots");
pg_log_warning_detail("Subscriptions will be created with the
two_phase option disabled. "
"Prepared transactions will be
replicated at COMMIT PREPARED.");
pg_log_warning_hint("You can use '--enable-two_phase' switch
to enable two_phase.");
}

~~~

But, check what other people think just in case I am in the minority
by thinking these warnings in docs/code are still potentially useful.

======
[1] https://www.postgresql.org/message-id/CAFPTHDbdXa4wh01B98L8VssJnyH%3DuK6Qfi%3DsKKVXRq-9_YwXsg%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia