On Fri, Mar 4, 2022 at 2:55 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Mar 4, 2022 at 6:40 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > Attached updated version patches. > > > > The patch looks mostly good to me. Few minor comments:
Thank you for the comments! > 1. I think we can have an Assert for errarg->origin_name in > apply_error_callback after checking the command as this function > assumes that it will always be set. Added. > 2. I suggest minor changes in the documentation change: > When a conflict produces an error, the replication won't proceed, and > the apply worker will emit the following kind of message to the > subscriber's server log: > +<screen> > +ERROR: duplicate key value violates unique constraint "test_pkey" > +DETAIL: Key (c)=(1) already exists. > +CONTEXT: processing remote data during "INSERT" for replication > target relation "public.test" in transaction 725 committed at LSN > 0/14BFA88 from replication origin "pg_16395" > +</screen> > > The LSN of the transaction that contains the change violating the > constraint and the replication origin name can be found from the > server log (LSN 0/14C0378 and replication origin > <literal>pg_16395</literal> in the above case). To skip the > transaction, the subscription needs to be disabled temporarily by > <command>ALTER SUBSCRIPTION ... DISABLE</command> first. Then, the > transaction can be skipped by calling the <link > linkend="pg-replication-origin-advance"> > <function>pg_replication_origin_advance()</function></link> function > with the <parameter>node_name</parameter> (i.e., > <literal>pg_16395</literal>) and the next LSN of the commit LSN (i.e., > LSN 0/14C0379). Fixed. On Fri, Mar 4, 2022 at 3:21 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Mar 4, 2022 at 11:45 AM osumi.takami...@fujitsu.com > <osumi.takami...@fujitsu.com> wrote: > > > > On Friday, March 4, 2022 2:23 PM Masahiko Sawada <sawada.m...@gmail.com> > > wrote: > > > I've attached updated patches. > > Hi, thank you for updating the patch. > > > > One comment on v4. > > > > In v4-0002, we introduce 'commit_lsn' in the ApplyErrorCallbackArg. > > This member is set for prepare, rollback prepared and stream_abort as well. > > The new log message format is useful when we have a prepare transaction > > that keeps failing on the subscriber and want to know the target transaction > > for the pg_replication_origin_advance(), right ? > > If this is true, I wasn't sure if the name 'commit_lsn' is the > > most accurate name for this variable. Should we adjust the name a bit ? > > > > If we want to change this variable name, the other options could be > 'end_lsn', or 'finish_lsn'. > Agreed with 'finish_lsn'. I've attached updated patches. Please review them. Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/
v4-0002-Add-the-origin-name-and-remote-commit-LSN-to-logi.patch
Description: Binary data
v4-0001-Use-complete-sentences-in-logical-replication-wor.patch
Description: Binary data
