On Fri, Nov 19, 2021 at 9:22 AM Amit Kapila <[email protected]> wrote:
>
> On Thu, Nov 18, 2021 at 5:10 PM Masahiko Sawada <[email protected]> wrote:
> >
> > On Thu, Nov 18, 2021 at 5:45 PM [email protected]
> > <[email protected]> wrote:
> > >
> > > On Tuesday, November 16, 2021 2:31 PM Masahiko Sawada
> > > <[email protected]> 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
