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. -- With Regards, Amit Kapila.
