Re: pgstat include expansion

On Tue, Feb 17, 2026 at 12:30 PM Nisha Moond <nisha(dot)moond412(at)gmail(dot)com> wrote:
>
> 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:
> > >
> > >
> > > 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].
>

*
@@ -5606,8 +5606,10 @@ start_apply(XLogRecPtr origin_startpos)
* idle state.
*/
AbortOutOfAnyTransaction();
- pgstat_report_subscription_error(MySubscription->oid,
- MyLogicalRepWorker->type);
+ if (am_tablesync_worker())
+ pgstat_report_subscription_tablesync_error(MySubscription->oid);
+ else
+ pgstat_report_subscription_apply_error(MySubscription->oid);

PG_RE_THROW();
}
@@ -5960,8 +5962,12 @@ DisableSubscriptionAndExit(void)
* Report the worker failed during sequence synchronization, table
* synchronization, or apply.
*/
- pgstat_report_subscription_error(MyLogicalRepWorker->subid,
- MyLogicalRepWorker->type);
+ if (am_tablesync_worker())
+ pgstat_report_subscription_tablesync_error(MySubscription->oid);
+ else if (am_sequencesync_worker())
+ pgstat_report_subscription_sequencesync_error(MySubscription->oid);
+ else
+ pgstat_report_subscription_apply_error(MySubscription->oid);

As the worker type is not directly available in all places, the other
possibility is to expose a function like GetLogicalWorkerType from
worker_internal.h and use it directly in
pgstat_report_subscription_error() instead of passing it via
parameter. We can get the worker_type via MyLogicalRepWorker->type
which should be accessible as it will be invoked by one of the logical
replication workers.

--
With Regards,
Amit Kapila.