pgsql: Add ALTER SUBSCRIPTION ... SKIP.

Amit Kapila <akapila@postgresql.org> writes:

The user can specify LSN by ALTER SUBSCRIPTION ... SKIP (lsn = XXX),
which allows the apply worker to skip the transaction finished at
specified LSN. The apply worker skips all data modification changes within
the transaction.

Hmm ... this seems like a really poor choice of syntax.
I would expect ALTER to be used for changes of persistent
object properties, which surely this is not?

An alternative perhaps could be to invoke the operation
via a function.

regards, tom lane

On Tue, Mar 22, 2022 at 7:36 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

Amit Kapila <akapila@postgresql.org> writes:

The user can specify LSN by ALTER SUBSCRIPTION ... SKIP (lsn = XXX),
which allows the apply worker to skip the transaction finished at
specified LSN. The apply worker skips all data modification changes within
the transaction.

Hmm ... this seems like a really poor choice of syntax.
I would expect ALTER to be used for changes of persistent
object properties, which surely this is not?

We have discussed this syntax and discussed the point that this is
different from other properties of subscription like slot_name, binary
etc. and that is why we used SKIP for it rather than the usual way by
using SET [1]/messages/by-id/CAA4eK1KD_C_0LSxaYB0UbG59VOgjf4mXBeSYbVWCLXAnnuqnPw@mail.gmail.com[2]/messages/by-id/f716f584-65d0-fe83-2e84-53426631739a@enterprisedb.com. There could also be other such options in future
like XID or other attributes, so we thought it would be easier to
extend it.

An alternative perhaps could be to invoke the operation
via a function.

I agree that is another alternative but could be inconvenient if there
are multiple such functions. We already have one
pg_replication_origin_advance().

[1]: /messages/by-id/CAA4eK1KD_C_0LSxaYB0UbG59VOgjf4mXBeSYbVWCLXAnnuqnPw@mail.gmail.com
[2]: /messages/by-id/f716f584-65d0-fe83-2e84-53426631739a@enterprisedb.com

--
With Regards,
Amit Kapila.

Hi,

On 2022-03-22 01:56:03 +0000, Amit Kapila wrote:

Add ALTER SUBSCRIPTION ... SKIP.

This feature allows skipping the transaction on subscriber nodes.

If incoming change violates any constraint, logical replication stops
until it's resolved. Currently, users need to either manually resolve the
conflict by updating a subscriber-side database or by using function
pg_replication_origin_advance() to skip the conflicting transaction. This
commit introduces a simpler way to skip the conflicting transactions.

The user can specify LSN by ALTER SUBSCRIPTION ... SKIP (lsn = XXX),
which allows the apply worker to skip the transaction finished at
specified LSN. The apply worker skips all data modification changes within
the transaction.

This was missing an include of xlogdefs.h in pg_subscription.h, thus failing
in headerscheck. See e.g.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&amp;dt=2022-03-22%2022%3A22%3A05

I've pushed the trivial fix for that. I'll propose adding headerscheck to CI /
cfbot.

Greetings,

Andres Freund

On Wed, Mar 23, 2022 at 5:29 AM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2022-03-22 01:56:03 +0000, Amit Kapila wrote:

Add ALTER SUBSCRIPTION ... SKIP.

This feature allows skipping the transaction on subscriber nodes.

If incoming change violates any constraint, logical replication stops
until it's resolved. Currently, users need to either manually resolve the
conflict by updating a subscriber-side database or by using function
pg_replication_origin_advance() to skip the conflicting transaction. This
commit introduces a simpler way to skip the conflicting transactions.

The user can specify LSN by ALTER SUBSCRIPTION ... SKIP (lsn = XXX),
which allows the apply worker to skip the transaction finished at
specified LSN. The apply worker skips all data modification changes within
the transaction.

This was missing an include of xlogdefs.h in pg_subscription.h, thus failing
in headerscheck. See e.g.
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=crake&amp;dt=2022-03-22%2022%3A22%3A05

I've pushed the trivial fix for that. I'll propose adding headerscheck to CI /
cfbot.

Thanks.

--
With Regards,
Amit Kapila.

On 2022-Mar-22, Amit Kapila wrote:

Add ALTER SUBSCRIPTION ... SKIP.

There are two messages here that seem oddly worded.

msgid "start skipping logical replication transaction finished at %X/%X"
msgid "done skipping logical replication transaction finished at %X/%X"

Two complaints here. First, the phrases "start / finished" and "done /
finished" look very strange. It took me a while to realize that
"finished" refers to the LSN, not to the skipping operation. Do we ever
talk about a transaction "finished at XYZ" as opposed to a transaction
whose LSN is XYZ? (This became particularly strange when I realized
that the LSN might come from a PREPARE.)

Second, "logical replication transaction". Is it not a regular
transaction that we happen to be processing via logical replication?

I think they should say something like

"logical replication starts skipping transaction with LSN %X/%X"
"logical replication completed skipping transaction with LSN %X/%X"

Other ideas?

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
Officer Krupke, what are we to do?
Gee, officer Krupke, Krup you! (West Side Story, "Gee, Officer Krupke")

On Sun, Sep 4, 2022 at 1:48 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2022-Mar-22, Amit Kapila wrote:

Add ALTER SUBSCRIPTION ... SKIP.

There are two messages here that seem oddly worded.

msgid "start skipping logical replication transaction finished at %X/%X"
msgid "done skipping logical replication transaction finished at %X/%X"

Two complaints here. First, the phrases "start / finished" and "done /
finished" look very strange. It took me a while to realize that
"finished" refers to the LSN, not to the skipping operation. Do we ever
talk about a transaction "finished at XYZ" as opposed to a transaction
whose LSN is XYZ? (This became particularly strange when I realized
that the LSN might come from a PREPARE.)

The reason to add "finished at ..." was to be explicit about whether
it is a starting LSN or an end LSN of a transaction. We do have such
differentiation in ReorderBufferTXN (first_lsn ... end_lsn).

Second, "logical replication transaction". Is it not a regular
transaction that we happen to be processing via logical replication?

I think they should say something like

"logical replication starts skipping transaction with LSN %X/%X"
"logical replication completed skipping transaction with LSN %X/%X"

This looks better to me. If you find the above argument to
differentiate between the start and end LSN convincing then we can
think of replacing "with" in the above messages with "finished at". I
see your point related to using "finished at" for PREPARE may not be a
good idea but I don't have better ideas for the same.

--
With Regards,
Amit Kapila.

On Sun, Sep 4, 2022 at 8:39 PM Amit Kapila <amit.kapila16@gmail.com> wrote:

On Sun, Sep 4, 2022 at 1:48 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:

On 2022-Mar-22, Amit Kapila wrote:

Add ALTER SUBSCRIPTION ... SKIP.

There are two messages here that seem oddly worded.

msgid "start skipping logical replication transaction finished at %X/%X"
msgid "done skipping logical replication transaction finished at %X/%X"

Two complaints here. First, the phrases "start / finished" and "done /
finished" look very strange. It took me a while to realize that
"finished" refers to the LSN, not to the skipping operation. Do we ever
talk about a transaction "finished at XYZ" as opposed to a transaction
whose LSN is XYZ? (This became particularly strange when I realized
that the LSN might come from a PREPARE.)

The reason to add "finished at ..." was to be explicit about whether
it is a starting LSN or an end LSN of a transaction. We do have such
differentiation in ReorderBufferTXN (first_lsn ... end_lsn).

Second, "logical replication transaction". Is it not a regular
transaction that we happen to be processing via logical replication?

I think they should say something like

"logical replication starts skipping transaction with LSN %X/%X"
"logical replication completed skipping transaction with LSN %X/%X"

This looks better to me.

+1

If you find the above argument to
differentiate between the start and end LSN convincing then we can
think of replacing "with" in the above messages with "finished at". I
see your point related to using "finished at" for PREPARE may not be a
good idea but I don't have better ideas for the same.

Given that the user normally doesn't need to be aware of the
difference between start LSN and end LSN in the context of using this
feature, I think we can use "with LSN %X/%X".

Regards,

--
Masahiko Sawada

On Tue, Sep 6, 2022 at 7:40 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:

I think they should say something like

"logical replication starts skipping transaction with LSN %X/%X"
"logical replication completed skipping transaction with LSN %X/%X"

This looks better to me.

+1

If you find the above argument to
differentiate between the start and end LSN convincing then we can
think of replacing "with" in the above messages with "finished at". I
see your point related to using "finished at" for PREPARE may not be a
good idea but I don't have better ideas for the same.

Given that the user normally doesn't need to be aware of the
difference between start LSN and end LSN in the context of using this
feature, I think we can use "with LSN %X/%X".

Fair enough.

Alvaro, would you like to push your proposed change? Otherwise, I am
happy to take care of this.

--
With Regards,
Amit Kapila.