On Thu, Dec 16, 2021 at 11:43 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Wed, Dec 15, 2021 at 8:19 PM Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > > On Wed, Dec 15, 2021 at 1:10 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > > > On Wed, Dec 15, 2021 at 8:19 AM Masahiko Sawada <sawada.m...@gmail.com> > > > wrote: > > > > > > > > On Tue, Dec 14, 2021 at 2:35 PM Greg Nancarrow <gregn4...@gmail.com> > > > > wrote: > > > > > > > > > > On Tue, Dec 14, 2021 at 3:23 PM vignesh C <vignes...@gmail.com> wrote: > > > > > > > > > > > > While the worker is skipping one of the skip transactions specified > > > > > > by > > > > > > the user and immediately if the user specifies another skip > > > > > > transaction while the skipping of the transaction is in progress > > > > > > this > > > > > > new value will be reset by the worker while clearing the skip xid. I > > > > > > felt once the worker has identified the skip xid and is about to > > > > > > skip > > > > > > the xid, the worker can acquire a lock to prevent concurrency > > > > > > issues: > > > > > > > > > > That's a good point. > > > > > If only the last_error_xid could be skipped, then this wouldn't be an > > > > > issue, right? > > > > > If a different xid to skip is specified while the worker is currently > > > > > skipping a transaction, should that even be allowed? > > > > > > > > > > > > > We don't expect such usage but yes, it could happen and seems not > > > > good. I thought we can acquire Share lock on pg_subscription during > > > > the skip but not sure it's a good idea. It would be better if we can > > > > find a way to allow users to specify only XID that has failed. > > > > > > > > > > Yeah, but as we don't have a definite way to allow specifying only > > > failed XID, I think it is better to use share lock on that particular > > > subscription. We are already using it for add/update rel state (see, > > > AddSubscriptionRelState, UpdateSubscriptionRelState), so this will be > > > another place to use a similar technique. > > > > Yes, but it seems to mean that we disallow users to change skip_xid > > while the apply worker is skipping changes so we will end up having > > the same problem we discussed so far; > > > > I thought we just want to lock before clearing the skip_xid something > like take the lock, check if the skip_xid in the catalog is the same > as we have skipped, if it is the same then clear it, otherwise, leave > it as it is. How will that disallow users to change skip_xid when we > are skipping changes?
Oh I thought we wanted to keep holding the lock while skipping changes (changing skip_xid requires acquiring the lock). So if skip_xid is already changed, the apply worker would do replorigin_advance() with WAL logging, instead of committing the catalog change? Regards, -- Masahiko Sawada EDB: https://www.enterprisedb.com/