On Wed, Jan 19, 2022 at 5:58 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Jan 19, 2022 at 12:46 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > > On Wed, Jan 19, 2022 at 12:22 PM osumi.takami...@fujitsu.com > > <osumi.takami...@fujitsu.com> wrote: > > > > > > (6) apply_handle_stream_abort > > > > > > @@ -1209,6 +1300,13 @@ apply_handle_stream_abort(StringInfo s) > > > > > > logicalrep_read_stream_abort(s, &xid, &subxid); > > > > > > + /* > > > + * We don't expect the user to set the XID of the transaction > > > that is > > > + * rolled back but if the skip XID is set, clear it. > > > + */ > > > + if (MySubscription->skipxid == xid || MySubscription->skipxid == > > > subxid) > > > + clear_subscription_skip_xid(MySubscription->skipxid, > > > InvalidXLogRecPtr, 0); > > > + > > > > > > In my humble opinion, this still cares about subtransaction xid still. > > > If we want to be consistent with top level transactions only, > > > I felt checking MySubscription->skipxid == xid should be sufficient. > > > > I thought if we can clear subskipxid whose value has already been > > processed on the subscriber with a reasonable cost it makes sense to > > do that because it can reduce the possibility of the issue that XID is > > wraparound while leaving the wrong in subskipxid. > > > > I guess that could happen if the user sets some unrelated XID value. > So, I think it should be okay to not clear this but we can add a > comment in the code at that place that we don't clear subtransaction's > XID as we don't support skipping individual subtransactions or > something like that.
Agreed and added the comment in the latest patch[1]. Regards, [1] https://www.postgresql.org/message-id/CAD21AoDOuNtvFUfU2wH2QgTJ6AyMXXh_vdA87qX0mUibdsrYTg%40mail.gmail.com -- Masahiko Sawada EDB: https://www.enterprisedb.com/
