Dear Nathan,
Thank you for updating the patch!
> In v3, I moved the call to LogicalRepWorkersWakeupAtCommit() to the end of
> the function. This should avoid waking up workers in some cases where it's
> unnecessary (e.g., if ALTER SUBSCRIPTION ERRORs in a subtransaction), but
> there are still cases where we'll wake up the workers unnecessarily. I
> think this is unlikely to cause any real problems in practice.
I understood you could accept false-positive event to avoid missing true-negative
like ALTER SUBSCRIPTION REFRESH. +1.
> >> 02. LogicalRepWorkersWakeupAtCommit()
> >>
> >> ```
> >> + oldcxt = MemoryContextSwitchTo(TopTransactionContext);
> >> + on_commit_wakeup_workers_subids =
> >> lappend_oid(on_commit_wakeup_workers_subids,
> >> +
> >> subid);
> >> ```
> >>
> >> If the subscription is altered twice in the same transaction, the same subid will
> >> be recorded twice.
> >> I'm not sure whether it may be caused some issued, but list_member_oid() can
> >> be used to avoid that.
> >
> > +1, list_append_unique_oid might be better.
>
> Done in v3.
I have no comments for the v3 patch.
Best Regards,
Hayato Kuroda
FUJITSU LIMITED