Re: Skipping logical replication transactions on subscriber side - Mailing list pgsql-hackers
From | Masahiko Sawada |
---|---|
Subject | Re: Skipping logical replication transactions on subscriber side |
Date | |
Msg-id | CAD21AoCdczrC_xOk2Anx=PeszgxxiWO5yaPztaEhiv9f6AzZig@mail.gmail.com Whole thread Raw |
In response to | Re: Skipping logical replication transactions on subscriber side (Amit Kapila <amit.kapila16@gmail.com>) |
Responses |
RE: Skipping logical replication transactions on subscriber side
("osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>)
RE: Skipping logical replication transactions on subscriber side ("shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>) RE: Skipping logical replication transactions on subscriber side ("osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>) |
List | pgsql-hackers |
On Thu, Mar 10, 2022 at 9:02 PM Amit Kapila <amit.kapila16@gmail.com> wrote: > > On Tue, Mar 1, 2022 at 8:31 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > > > I've attached an updated patch along with two patches for cfbot tests > > since the main patch (0003) depends on the other two patches. Both > > 0001 and 0002 patches are the same ones I attached on another > > thread[2]. > > > > Few comments on 0003: > ===================== > 1. > + <row> > + <entry role="catalog_table_entry"><para role="column_definition"> > + <structfield>subskiplsn</structfield> <type>pg_lsn</type> > + </para> > + <para> > + Commit LSN of the transaction whose changes are to be skipped, > if a valid > + LSN; otherwise <literal>0/0</literal>. > + </para></entry> > + </row> > > Can't this be prepared LSN or rollback prepared LSN? Can we say > Finish/End LSN and then add some details which all LSNs can be there? Right, changed to finish LSN. > > 2. The conflict resolution explanation needs an update after the > latest commits and we should probably change the commit LSN > terminology as mentioned in the previous point. Updated. > > 3. The text in alter_subscription.sgml looks a bit repetitive to me > (similar to what we have in logical-replication.sgml related to > conflicts). Here also we refer to only commit LSN which needs to be > changed as mentioned in the previous two points. Updated. > > 4. > if (strcmp(lsn_str, "none") == 0) > + { > + /* Setting lsn = NONE is treated as resetting LSN */ > + lsn = InvalidXLogRecPtr; > + } > + else > + { > + /* Parse the argument as LSN */ > + lsn = DatumGetTransactionId(DirectFunctionCall1(pg_lsn_in, > + CStringGetDatum(lsn_str))); > + > + if (XLogRecPtrIsInvalid(lsn)) > + ereport(ERROR, > + (errcode(ERRCODE_INVALID_PARAMETER_VALUE), > + errmsg("invalid WAL location (LSN): %s", lsn_str))); > > Is there a reason that we don't want to allow setting 0 > (InvalidXLogRecPtr) for skip LSN? 0 is obviously an invalid value for skip LSN, which should not be allowed similar to other options (like setting '' to slot_name). Also, we use 0 (InvalidXLogRecPtr) internally to reset the subskipxid when NONE is specified. > > 5. > +# The subscriber will enter an infinite error loop, so we don't want > +# to overflow the server log with error messages. > +$node_subscriber->append_conf( > + 'postgresql.conf', > + qq[ > +wal_retrieve_retry_interval = 2s > +]); > > Can we change this test to use disable_on_error feature? I am thinking > if the disable_on_error feature got committed first, maybe we can have > one test file for this and disable_on_error feature (something like > conflicts.pl). Good idea. Updated. I've attached an updated version patch. This patch can be applied on top of the latest disable_on_error patch[1]. Regards, [1] https://www.postgresql.org/message-id/CAA4eK1Kes9TsMpGL6m%2BAJNHYCGRvx6piYQt5v6TEbH_t9jh8nA%40mail.gmail.com -- Masahiko Sawada EDB: https://www.enterprisedb.com/
Attachment
pgsql-hackers by date: