RE: Skipping logical replication transactions on subscriber side - Mailing list pgsql-hackers

On Thursday, August 12, 2021 1:53 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> I've attached the updated patches. FYI I've included the patch
> (v8-0005) that fixes the assertion failure during shared fileset
> cleanup to make cfbot tests happy.


Hi

Thanks for your patch. I met a problem when using it. The log is not what I expected in some cases, but in streaming
mode,they work well.
 

For example:
------publisher------
create table test (a int primary key, b varchar);
create publication pub for table test;

------subscriber------
create table test (a int primary key, b varchar);
insert into test values (10000);
create subscription sub connection 'dbname=postgres port=5432' publication pub with(streaming=on);

------publisher------
insert into test values (10000);

Subscriber log:
2021-08-17 14:24:43.415 CST [3630341] ERROR:  duplicate key value violates unique constraint "test_pkey"
2021-08-17 14:24:43.415 CST [3630341] DETAIL:  Key (a)=(10000) already exists.

It didn't give more context info generated by apply_error_callback function.

In streaming mode(which worked as I expected):
------publisher------
INSERT INTO test SELECT i, md5(i::text) FROM generate_series(1, 10000) s(i);

Subscriber log:
2021-08-17 14:26:26.521 CST [3630510] ERROR:  duplicate key value violates unique constraint "test_pkey"
2021-08-17 14:26:26.521 CST [3630510] DETAIL:  Key (a)=(10000) already exists.
2021-08-17 14:26:26.521 CST [3630510] CONTEXT:  processing remote data during "INSERT" for replication target relation
"public.test"in transaction id 710 with commit timestamp 2021-08-17 14:26:26.403214+08
 

I looked into it briefly and thought it was related to some code in
apply_dispatch function. It set callback when apply_error_callback_arg.command
is 0, and reset the callback back at the end of the function. But
apply_error_callback_arg.command was not reset to 0, so it won't set callback
when calling apply_dispatch function next time.

I tried to fix it with the following change, thoughts?

@@ -2455,7 +2455,10 @@ apply_dispatch(StringInfo s)

        /* Pop the error context stack */
        if (set_callback)
+       {
                error_context_stack = errcallback.previous;
+               apply_error_callback_arg.command = 0;
+       }
 }

Besides, if we make the changes like this, do we still need to reset
apply_error_callback_arg.command in reset_apply_error_context_info function?

Regards
Tang