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: Greg Nancarrow <gregn4422@gmail.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, "osumi.takamichi@fujitsu.com" <osumi.takamichi@fujitsu.com>, Alexey Lesovsky <lesovsky@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2021-10-12T04:59:23Z
Lists: pgsql-hackers

Attachments

On Fri, Oct 8, 2021 at 8:17 PM Greg Nancarrow <gregn4422@gmail.com> wrote:
>
> On Thu, Sep 30, 2021 at 3:45 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached updated patches that incorporate all comments I got so
> > far. Please review them.
> >
>
> Some comments about the v15-0001 patch:

Thank you for the comments!

>
> (1) patch adds a whitespace error
>
> Applying: Add a subscription errors statistics view
> "pg_stat_subscription_errors".
> .git/rebase-apply/patch:1656: new blank line at EOF.
> +
> warning: 1 line adds whitespace errors.

Fixed.

>
> (2) Patch comment says "This commit adds a new system view
> pg_stat_logical_replication_errors ..."
> BUT this is the wrong name, it should be "pg_stat_subscription_errors".
>
>

Fixed.

> doc/src/sgml/monitoring.sgml
>
> (3)
> "Message of the error" doesn't sound right. I suggest just saying "The
> error message".

Fixed.

>
> (4) view column "last_failed_time"
> I think it would be better to name this "last_error_time".

Okay, fixed.

>
>
> src/backend/postmaster/pgstat.c
>
> (5) pgstat_vacuum_subworker_stats()
>
> Spelling mistake in the following comment:
>
> /* Create a map for mapping subscriptoin OID and database OID */
>
> subscriptoin -> subscription

Fixed.

>
> (6)
> In the following functions:
>
> pgstat_read_statsfiles
> pgstat_read_db_statsfile_timestamp
>
> The following comment should say "... struct describing subscription
> worker statistics."
> (i.e. need to remove the "a")
>
> + * 'S' A PgStat_StatSubWorkerEntry struct describing a
> + * subscription worker statistics.
>

Fixed.

>
> (7) pgstat_get_subworker_entry
>
> Suggest comment change:
>
> BEFORE:
> + * Return the entry of subscription worker entry with the subscription
> AFTER:
> + * Return subscription worker entry with the given subscription

Fixed.

>
> (8) pgstat_recv_subworker_error
>
> + /*
> + * Update only the counter and timestamp if we received the same error
> + * again
> + */
> + if (wentry->relid == msg->m_relid &&
> + wentry->command == msg->m_command &&
> + wentry->xid == msg->m_xid &&
> + strncmp(wentry->message, msg->m_message, strlen(wentry->message)) == 0)
> + {
>
> Is there a reason that the above check uses strncmp() with
> strlen(wentry->message), instead of just strcmp()?
> msg->m_message is treated as the same error message if it is the same
> up to strlen(wentry->message)?
> Perhaps if that is intentional, then the comment should be updated.

It's better to use strcmp() in this case. Fixed.

>
> src/tools/pgindent/typedefs.list
>
> (9)
> The added "PgStat_SubWorkerError" should be removed from the
> typedefs.list (as there is no such new typedef).

Fixed.

I've attached updated patches.

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