Horton

Re: Skipping logical replication transactions on subscriber side

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Alexey Lesovsky <lesovsky@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-08-10T10:18:15Z
Lists: pgsql-hackers 
On Tue, Aug 10, 2021 at 11:59 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Aug 10, 2021 at 10:37 AM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached the latest patches that incorporated all comments I got
> > so far. Please review them.
> >
>
> I am not able to apply the latest patch
> (v6-0001-Add-errcontext-to-errors-happening-during-applyin) on HEAD,
> getting the below error:
>

Few comments on v6-0001-Add-errcontext-to-errors-happening-during-applyin
==============================================================

1. While applying DML operations, we are setting up the error context
multiple times due to which the context information is not
appropriate. The first is set in apply_dispatch and then during
processing, we set another error callback slot_store_error_callback in
slot_store_data and slot_modify_data. When I forced one of the errors
in slot_store_data(), it displays the below information in CONTEXT
which doesn't make much sense.

2021-08-10 15:16:39.887 IST [6784] ERROR:  incorrect binary data
format in logical replication column 1
2021-08-10 15:16:39.887 IST [6784] CONTEXT:  processing remote data
for replication target relation "public.test1" column "id"
        during apply of "INSERT" for relation "public.test1" in
transaction with xid 740 committs 2021-08-10 14:44:38.058174+05:30

2.
I think we can slightly change the new context information as below:
Before
during apply of "INSERT" for relation "public.test1" in transaction
with xid 740 committs 2021-08-10 14:44:38.058174+05:30
After
during apply of "INSERT" for relation "public.test1" in transaction id
740 with commit timestamp 2021-08-10 14:44:38.058174+05:30


3.
+/* Struct for saving and restoring apply information */
+typedef struct ApplyErrCallbackArg
+{
+ LogicalRepMsgType command; /* 0 if invalid */
+
+ /* Local relation information */
+ char    *nspname;
+ char    *relname;

...
...

+
+static ApplyErrCallbackArg apply_error_callback_arg =
+{
+ .command = 0,
+ .relname = NULL,
+ .nspname = NULL,

Let's initialize the struct members in the order they are declared.
The order of relname and nspname should be another way.

4.
+
+ TransactionId remote_xid;
+ TimestampTz committs;
+} ApplyErrCallbackArg;

It might be better to add a comment like "remote xact information"
above these structure members.

5.
+static void
+apply_error_callback(void *arg)
+{
+ StringInfoData buf;
+
+ if (apply_error_callback_arg.command == 0)
+ return;
+
+ initStringInfo(&buf);

At the end of this call, it is better to free this (pfree(buf.data))

6. In the commit message, you might want to indicate that this
additional information can be used by the future patch to skip the
conflicting transaction.


-- 
With Regards,
Amit Kapila.

Commits

  1. Test ALIGNOF_DOUBLE==4 compatibility under ALIGNOF_DOUBLE==8.

  2. Reorder subskiplsn in pg_subscription to avoid alignment issues.

  3. Add ALTER SUBSCRIPTION ... SKIP.

  4. Optionally disable subscriptions on error.

  5. Update docs of logical replication for commit 8d74fc96db.

  6. Respect permissions within logical replication.

  7. Fix regression test failure caused by commit 8d74fc96db.

  8. Add a view to show the stats of subscription workers.

  9. Add logical change details to logical replication worker errcontext.

  10. Rename LOGICAL_REP_MSG_STREAM_END to LOGICAL_REP_MSG_STREAM_STOP.

  11. Fix typo in protocol.sgml.

  12. Remove unused argument in apply_handle_commit_internal().

  13. Fix replication of in-progress transactions in tablesync worker.

  14. Reorder pg_sequence columns to avoid alignment issue