Horton

Re: Skipping logical replication transactions on subscriber side

Masahiko Sawada <sawada.mshk@gmail.com>

View thread View on postgr.es
From: Masahiko Sawada <sawada.mshk@gmail.com>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, Greg Nancarrow <gregn4422@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, "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-09-24T01:31:09Z
Lists: pgsql-hackers 
Hi,

On Fri, Sep 3, 2021 at 4:33 AM Mark Dilger <mark.dilger@enterprisedb.com> wrote:
>
>
>
> > On Aug 30, 2021, at 12:06 AM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached rebased patches.
>
> Thanks for these patches, Sawada-san!

Sorry for the very late response.

Thank you for the suggestions and the patch!

>
> The first patch in your series, v12-0001, seems useful to me even before committing any of the rest.  I would like to integrate the new pg_stat_subscription_errors view it creates into regression tests for other logical replication features under development.
>
> In particular, it can be hard to write TAP tests that need to wait for subscriptions to catch up or fail.  With your view committed, a new PostgresNode function to wait for catchup or for failure can be added, and then developers of different projects can all use that.

I like the idea of creating a common function that waits for the
subscription to be ready (i.e., all relations are either in 'r' or 's'
state). There are many places where we wait for all subscription
relations to be ready in existing tap tests. We would be able to
replace those codes with the function. But I'm not sure that it's
useful to have a function that waits for the subscriptions to either
be ready or raise an error. In tap tests, I think that if we wait for
the subscription to raise an error, we should wait only for the error
but not for the subscription to be ready. Thoughts?

> I am attaching a version of such a function, plus some tests of your patch (since it does not appear to have any).  Would you mind reviewing these and giving comments or including them in your next patch version?
>

I've looked at the patch and here are some comments:

+
+-- no errors should be reported
+SELECT * FROM pg_stat_subscription_errors;
+

+
+-- Test that the subscription errors view exists, and has the right columns
+-- If we expected any rows to exist, we would need to filter out unstable
+-- columns.  But since there should be no errors, we just select them all.
+select * from pg_stat_subscription_errors;

The patch adds checks of pg_stat_subscription_errors in order to test
if the subscription doesn't have any error. But since the subscription
errors are updated in an asynchronous manner, we cannot say the
subscription is working fine by checking the view only once.

---
The newly added tap tests by 025_errors.pl have two subscribers raise
a table sync error, which seems very similar to the tests that
024_skip_xact.pl adds. So I'm not sure we need this tap test as a
separate tap test file.

Regards,

-- 
Masahiko Sawada
EDB:  https://www.enterprisedb.com/

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