public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Kapila <[email protected]>
To: [email protected] <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: [email protected] <[email protected]>
Cc: [email protected] <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Perform streaming logical transactions by background workers and parallel apply
Date: Tue, 28 Jun 2022 09:45:10 +0530
Message-ID: <CAA4eK1Lga3TuDGsO4xuYV_hTw=MQ2g-bK2QENTLuYpSuTeV4cQ@mail.gmail.com> (raw)
In-Reply-To: <OS3PR01MB62758DBE8FA12BA72A43AC819EB89@OS3PR01MB6275.jpnprd01.prod.outlook.com>
References: <CAA4eK1+wyN6zpaHUkCLorEWNx75MG0xhMwcFhvjqm2KURZEAGw@mail.gmail.com>
	<OS0PR01MB57160DFDEF5B1C4668346B7D94E99@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB57166BC3CAA873364A2CC07A94EF9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAA4eK1Kc0VAhJAQe4_5YGrnTpfKYA9=F6sxqEKPrrax5Hy0+jw@mail.gmail.com>
	<OS0PR01MB571616192F70A18AF88F3B2894F59@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB5716A6C18E69FB1EA9A27EED94F59@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CAHut+PuqYP5eD5wcSCtk=a6KuMjat2UCzqyGoE7sieCaBsVskQ@mail.gmail.com>
	<OS0PR01MB57160D1B0D29DC66ED035F5694F89@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OS0PR01MB5716E8D536552467EFB512EF94FC9@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<OSZPR01MB6310F0FABB05F8E5BB31A5B0FDFC9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<CAHut+Pv3FV+WibRBuGCyXd_ri+O4L3iY+UnHO2SHRbmC+xR6dw@mail.gmail.com>
	<OSZPR01MB63106EADF50E0E710D36CE5DFDCA9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
	<CAHut+PuAxW57fowiMrn=3=53sagmehiTSW0o1Q52MpR3phUmyw@mail.gmail.com>
	<OS3PR01MB6275797F66EF0A47EEB2D8FC9EDD9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1Jt08SYbRt_-rbSWNg=X9-m8+RdP5PosfnQgyF-z8bkxQ@mail.gmail.com>
	<CAA4eK1+Z6ahpTQK2KzkvQ1kN-urVS9-N_RDM11MS+btqaB8Bpw@mail.gmail.com>
	<CAD21AoC1ME-xQjdcTeMpVMADKJDjfGVZATdGNuR+Qcimov5TPA@mail.gmail.com>
	<CAA4eK1L=KsaU4fCpM_xbwK08E+NpYMviH23+uD0Be_2pKNtw_g@mail.gmail.com>
	<OS3PR01MB62758A881FF3240171B7B21B9EDE9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<OS3PR01MB6275208A2F8ED832710F65E09EA49@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<OS3PR01MB62756BD9482EB6BB1CA4CD4D9EAA9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1+GM6dmuFBnVGqiOs6UocN3c8mQZXD1Un=Sa2x_8vVfbw@mail.gmail.com>
	<OS3PR01MB62753CDFD0DF1FCBC978AFB39EAF9@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1+iiwpfmaoNPPktTkVTGjprv_Fjpr_fu7yoOUKEfDtv_A@mail.gmail.com>
	<OS3PR01MB6275F61CC74C5F3E47C7DD229EB59@OS3PR01MB6275.jpnprd01.prod.outlook.com>
	<CAA4eK1JD_vCurmjdVStbXoKn8Smz=3YcUA4-jWp4X5E8SgmP3A@mail.gmail.com>
	<OS3PR01MB62758DBE8FA12BA72A43AC819EB89@OS3PR01MB6275.jpnprd01.prod.outlook.com>

On Tue, Jun 28, 2022 at 8:51 AM [email protected]
<[email protected]> wrote:
>
> On Thu, Jun 23, 2022 at 16:44 PM Amit Kapila <[email protected]> wrote:
> > On Thu, Jun 23, 2022 at 12:51 PM [email protected]
> > <[email protected]> wrote:
> > >
> > > On Mon, Jun 20, 2022 at 11:00 AM Amit Kapila <[email protected]>
> > 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.
>
> Thanks for your comments.
> I improved it as you suggested. I mask the LSN if it is invalid LSN(0/0).
> Also, I improved the related pg-doc as following:
> ```
>    When the streaming mode is <literal>parallel</literal>, the finish LSN of
>    failed transactions may not be logged. In that case, it may be necessary to
>    change the streaming mode to <literal>on</literal> and cause the same
>    conflicts again so the finish LSN of the failed transaction will be written
>    to the server log. For the usage of finish LSN, please refer to <link
>    linkend="sql-altersubscription"><command>ALTER SUBSCRIPTION ...
>    SKIP</command></link>.
> ```
> After improving this (mask invalid LSN), I found that this improvement and
> parallel apply patch do not seem to have a strong correlation. Would it be
> better to improve and commit in another separate patch?
>

Is there any other case where we can hit this code path (mask
invalidLSN) without this patch?

-- 
With Regards,
Amit Kapila.





view thread (633+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Perform streaming logical transactions by background workers and parallel apply
  In-Reply-To: <CAA4eK1Lga3TuDGsO4xuYV_hTw=MQ2g-bK2QENTLuYpSuTeV4cQ@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox