On Thu, Jun 23, 2022 at 12:51 PM wangw.f...@fujitsu.com
<wangw.f...@fujitsu.com> wrote:
>
> On Mon, Jun 20, 2022 at 11:00 AM Amit Kapila <amit.kapil...@gmail.com> wrote:
> > I have improved the comments in this and other related sections of the
> > patch. See attached.
> Thanks for your comments and patch!
> Improved the comments as you suggested.
>
> > > > 3.
> > > > +
> > > > + <para>
> > > > + Setting streaming mode to <literal>apply</literal> could export
> > > > invalid
> > LSN
> > > > + as finish LSN of failed transaction. Changing the streaming mode and
> > making
> > > > + the same conflict writes the finish LSN of the failed transaction
> > > > in the
> > > > + server log if required.
> > > > + </para>
> > > >
> > > > How will the user identify that this is an invalid LSN value and she
> > > > shouldn't use it to SKIP the transaction? Can we change the second
> > > > sentence to: "User should change the streaming mode to 'on' if they
> > > > would instead wish to see the finish LSN on error. Users can use
> > > > finish LSN to SKIP applying the transaction." I think we can give
> > > > reference to docs where the SKIP feature is explained.
> > > Improved the sentence as suggested.
> > >
> >
> > You haven't answered first part of the comment: "How will the user
> > identify that this is an invalid LSN value and she shouldn't use it to
> > SKIP the transaction?". Have you checked what value it displays? For
> > example, in one of the case in apply_error_callback as shown in below
> > code, we don't even display finish LSN if it is invalid.
> > else if (XLogRecPtrIsInvalid(errarg->finish_lsn))
> > errcontext("processing remote data for replication origin \"%s\"
> > during \"%s\" in transaction %u",
> > errarg->origin_name,
> > logicalrep_message_type(errarg->command),
> > errarg->remote_xid);
> I am sorry that I missed something in my previous reply.
> The invalid LSN value here is to say InvalidXLogRecPtr (0/0).
> Here is an example :
> ```
> 2022-06-23 14:30:11.343 CST [822333] logical replication worker CONTEXT:
> processing remote data for replication origin "pg_16389" during "INSERT" for
> replication target relation "public.tab" in transaction 727 finished at 0/0
> ```
>
I don't think it is a good idea to display invalid values. We can mask
this as we are doing in other cases in function apply_error_callback.
The ideal way is that we provide a view/system table for users to
check these errors but that is a matter of another patch. So users
probably need to check Logs to see if the error is from a background
apply worker to decide whether or not to switch streaming mode.
--
With Regards,
Amit Kapila.
