On Tue, Feb 17, 2026 at 10:15 AM Amit Kapila <[email protected]> wrote: > > 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). > 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
v1-0001-Split-pgstat_report_subscription_error-by-worker-.patch
Description: Binary data
