On Fri, Nov 19, 2021 at 9:22 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Thu, Nov 18, 2021 at 5:10 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Thu, Nov 18, 2021 at 5:45 PM tanghy.f...@fujitsu.com > > <tanghy.f...@fujitsu.com> wrote: > > > > > > On Tuesday, November 16, 2021 2:31 PM Masahiko Sawada > > > <sawada.m...@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