Re: pgstat include expansion

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).

With Regards,
Amit Kapila.