Re: Skipping logical replication transactions on subscriber side

From: "David G(dot) Johnston" <david(dot)g(dot)johnston(at)gmail(dot)com>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>, Alexey Lesovsky <lesovsky(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Skipping logical replication transactions on subscriber side
Date: 2022-01-21 16:29:51
Message-ID: CAKFQuwZ=iZvnKeA+2-LWrYGOJqnDdb9rA7EqiVX0+9v=X3HCKA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 21, 2022 at 4:55 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:

> Apart from this, I have changed a few comments and ran pgindent. Do
> let me know what you think of the changes?
>

The paragraph describing ALTER SUBSCRIPTION SKIP seems unnecessarily
repetitive. Consider:
"""
Skips applying all changes of the specified remote transaction, whose value
should be obtained from pg_stat_subscription_workers.last_error_xid. While
this will result in avoiding the last error on the subscription, thus
allowing it to resume working. See "link to a more holistic description in
the Logical Replication chapter" for alternative means of resolving
subscription errors. Removing an entire transaction from the history of a
table should be considered a last resort as it can leave the system in a
very inconsistent state.

Note, this feature will not accept transactions prepared under two-phase
commit.

This command sets pg_subscription.subskipxid field upon issuance and the
system clears the same field upon seeing and successfully skipped the
identified transaction. Issuing this command again while a skipped
transaction is pending replaces the existing transaction with the new one.
"""

Then change the subskipxid column description to be:
"""
ID of the transaction whose changes are to be skipped. It is 0 when there
are no pending skips. This is set by issuing ALTER SUBSCRIPTION SKIP and
resets back to 0 when the identified transactions passes through the
subscription stream and is successfully ignored.
"""

I don't understand why/how ", if a valid transaction ID;" comes into play
(how would we know whether it is valid, or if we do ALTER SUBSCRIPTION SKIP
should prohibit the invalid value from being chosen).

I'm against mentioning subtransactions in the skip_option description.

The Logical Replication page changes provide good content overall but I
dislike going into detail about how to perform conflict resolution in the
third paragraph and then summarize the various forms of conflict resolution
in the newly added forth. Maybe re-work things like:

1. Logical replication behaves...
2. A conflict will produce...details can be found in places...
3. Resolving conflicts can be done by...
4. (split and reworded) If choosing to simply skip the offending
transaction you take the pg_stat_subscription_worker.last_error_xid value
(716 in the example above) and provide it while executing ALTER
SUBSCRIPTION SKIP...
5. (split and reworded) Prior to v15 ALTER SUBSCRIPTION SKIP was not
available and instead you had to use the pg_replication_origin_advance()
function...

Don't just list out two options for the user to perform the same action.
Tell a story about why we felt compelled to add ALTER SYSTEM SKIP and why
either the function is now deprecated or is useful given different
circumstances (the former seems likely).

David J.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2022-01-21 16:39:07 Re: New developer papercut - Makefile references INSTALL
Previous Message Julien Rouhaud 2022-01-21 16:13:31 Re: ICU for global collation