On Tue, Feb 17, 2026 at 10:15 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Feb 16, 2026 at 11:20 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
> >
> > I think we should simply not pass the worker type. The only reason the worker
> > type passed is that that is used as a proxy for what kind of error occurred:
> >
> >
> > /*
> > * Report a subscription error.
> > */
> > void
> > pgstat_report_subscription_error(Oid subid, int wtype)
> > {
> > PgStat_EntryRef *entry_ref;
> > PgStat_BackendSubEntry *pending;
> >
> > entry_ref = pgstat_prep_pending_entry(PGSTAT_KIND_SUBSCRIPTION,
> > InvalidOid, subid, NULL);
> > pending = entry_ref->pending;
> >
> > switch ((LogicalRepWorkerType) wtype)
> > {
> > case WORKERTYPE_APPLY:
> > pending->apply_error_count++;
> > break;
> >
> > case WORKERTYPE_SEQUENCESYNC:
> > pending->sync_seq_error_count++;
> > break;
> >
> > case WORKERTYPE_TABLESYNC:
> > pending->sync_table_error_count++;
> > break;
> >
> > default:
> > /* Should never happen. */
> > Assert(0);
> > break;
> > }
> > }
> >
> >
> > It doesn't seem like the right thing to have pgstat_subscription.c translate
> > the worker type to the concrete error this way. Afaict each of the callsites
> > of pgstat_report_subscription_error() actually knows what kind of error its
> > reporting, but just uses the worker type to do so.
> >
> > I'd either change the signature to have one argument for each of the error
> > types, i.e.
> > pgstat_report_subscription_error(int subid,
> > bool apply_error,
> > bool sequencesync_error,
> > bool_tablesync_error);
> >
> > or split the function into three, and have
> > pgstat_report_subscription_{apply,sequence,tablesync}(int subid);
> >
>
> Good idea. +1 for the second approach to split the function. We can
> name them as pgstat_report_subscription_apply_error(int subid),
> pgstat_report_subscription_sequence_error(int subid),
> pgstat_report_subscription_tablesync_error(int subid).
>
Please find the attached patch implementing the idea. Introduced three
new worker specific functions as suggested and includes a few required
fixups after removing worker_internal.h from pgstat.h, as done earlier
in Amit’s patch [1].
[1] https://www.postgresql.org/message-id/CAA4eK1%2BZOXJotnk%2Bd4mxHFP_i2SEWGTdyJgkQadOCvMgEcPGVg%40mail.gmail.com
--
Thanks,
Nisha