RE: Conflict detection for update_deleted in logical replication

On Saturday, January 18, 2025 11:45 AM Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com> wrote:
> 
> On Thursday, January 16, 2025 6:02 PM Amit Kapila
> <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Jan 15, 2025 at 2:20 PM Zhijie Hou (Fujitsu)
> > <houzj.fnst@fujitsu.com> wrote:
> > >
> > > In the latest version, we implemented a simpler approach that allows
> > > the apply worker to directly advance the oldest_nonremovable_xid if
> > > the waiting time exceeds the newly introduced option's limit. I've
> > > named this option "max_conflict_retention_duration," as it aligns
> > > better with the conflict detection concept and the "retain_conflict_info"
> > option.
> > >
> > > During the last phase (RCI_WAIT_FOR_LOCAL_FLUSH), the apply worker
> > > evaluates how much time it has spent waiting. If this duration
> > > exceeds the max_conflict_retention_duration, the worker directly
> > > advances the oldest_nonremovable_xid and logs a message indicating
> > > the forced advancement of the non-removable transaction ID.
> > >
> > > This approach is a bit like a time-based option that discussed before.
> > > Compared to the slot invalidation approach, this approach is simpler
> > > because we can avoid adding 1) new slot invalidation type due to
> > > apply lag, 2) new field lag_behind in shared memory
> > > (MyLogicalRepWorker) to indicate when the lag exceeds the limit, and
> > > 3) additional logic in the launcher to handle each worker's lag status.
> > >
> > > In the slot invalidation, user would be able to confirm if the
> > > current by checking if the slot in pg_replication_slot in
> > > invalidated or not, while in the simpler approach mentioned, user
> > > could only confirm that by
> > checking the LOGs.
> > >
> >
> > The user needs to check the LOGs corresponding to all subscriptions on
> > the node. I see the simplicity of the approach you used but still the
> > slot_invalidation idea sounds better to me on the grounds that it will
> > be convenient for users/DBA to know when to rely on the update_missing
> > type conflict if there is a valid and active slot with the name
> 'pg_conflict_detection'
> > (or whatever name we decide to give) then users can rely on the
> > detected conflict. Sawada-San, and others, do you have any preference on
> this matter?
> 
> I think invalidating the slot is OK and we could also let the apply worker to
> automatic recovery as suggested in [1].
> 
> Here is the V24 patch set. I modified 0004 patch to implement the slot
> Invalidation part. Since the automatic recovery could be an optimization and the
> discussion is in progress, I didn't implement that part.

The implementation is in progress and I will include it in next version.

Here is the V25 patch set that includes the following change:

0001

* Per off-list discussion with Amit, I added few comments to mention the
reason of skipping advancing xid when table sync is in progress and to mention
that the advancement will not be delayed if changes are filtered out on publisher
via row/table filter.

0004

* Fixed a bug that the launcher would advance the slot.xmin when some apply
  workers have not yet started.

* Fixed a bug that the launcher did not advance the slot.xmin even if one of the
  apply worker has stopped conflict retention due to the lag.

* Add a retain_conflict_info column in the pg_stat_subscription view to
  indicate whether the apply worker is effectively retaining conflict
  information. The value is set to true only if retain_conflict_info is enabled
  for the associated subscription, and the retention duration for conflict
  detection by the apply worker has not exceeded
  max_conflict_retention_duration. Thanks Kuroda-san for contributing codes
  off-list.

Best Regards,
Hou zj