On Tue, Feb 17, 2026 at 12:30 PM Nisha Moond <[email protected]> wrote:
>
> 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:
> > >
> > >
> > > 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.


Reply via email to