Re: Skipping logical replication transactions on subscriber side

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>, "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>, Greg Nancarrow <gregn4422@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.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-11-19T05:38:56Z
Lists: pgsql-hackers
On Fri, Nov 19, 2021 at 9:22 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Thu, Nov 18, 2021 at 5:10 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > On Thu, Nov 18, 2021 at 5:45 PM tanghy.fnst@fujitsu.com
> > <tanghy.fnst@fujitsu.com> wrote:
> > >
> > > On Tuesday, November 16, 2021 2:31 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > > >
> > > > Right. I've fixed this issue and attached an updated patch.
> > > >
> > > >
> > >
> > > Thanks for your patch.
> > >
> > > I read the discussion about stats entries for table sync worker[1], the
> > > statistics are retained after table sync worker finished its jobs and user can remove
> > > them via pg_stat_reset_subscription_worker function.
> > >
> > > But I notice that, if a table sync worker finished its jobs, the error reported by
> > > this worker will not be shown in the pg_stat_subscription_workers view. (It seemed caused by this condition: "WHERE srsubstate <> 'r'") Is it intentional? I think this may cause a result that users don't know the statistics are still exist, and won't remove the statistics manually. And that is not friendly to users' storage, right?
> > >
> >
> > You're right. The condition "WHERE substate <> 'r') should be removed.
> > I'll do that change in the next version patch. Thanks!
> >
>
> One more thing you might want to consider for the next version is
> whether to rename the columns as discussed in the related thread [1]?
> I think we should consider future work and name them accordingly.
>
> [1] - https://www.postgresql.org/message-id/CAA4eK1KR41bRUuPeNBSGv2%2Bq7ROKukS3myeAUqrZMD8MEwR0DQ%40mail.gmail.com

Since the statistics collector process uses UDP socket, the sequencing
of the messages is not guaranteed. Will there be a problem if
Subscription is dropped and stats collector receives
PGSTAT_MTYPE_SUBSCRIPTIONPURGE first and the subscription worker entry
is removed and then receives PGSTAT_MTYPE_SUBWORKERERROR(this order
can happen because of UDP socket). I'm not sure if the Assert will be
a problem in this case. If this scenario is possible we could just
silently return in that case.

+static void
+pgstat_recv_subworker_error(PgStat_MsgSubWorkerError *msg, int len)
+{
+       PgStat_StatDBEntry *dbentry;
+       PgStat_StatSubWorkerEntry *subwentry;
+
+       dbentry = pgstat_get_db_entry(msg->m_databaseid, true);
+
+       /* Get the subscription worker stats */
+       subwentry = pgstat_get_subworker_entry(dbentry, msg->m_subid,
+
            msg->m_subrelid, true);
+       Assert(subwentry);
+
+       /*
+        * Update only the counter and last error timestamp if we received
+        * the same error again
+        */

Thoughts?

Regards,
Vignesh



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