On Mon, Feb 16, 2026 at 11:20 PM Andres Freund <[email protected]> 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). With Regards, Amit Kapila.
