Re: Conflict detection and logging in logical replication

On Mon, Aug 5, 2024 at 10:05 AM shveta malik <shveta.malik@gmail.com> wrote:
>
> On Mon, Aug 5, 2024 at 9:19 AM Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Fri, Aug 2, 2024 at 6:28 PM Nisha Moond <nisha.moond412@gmail.com> wrote:
> > >
> > > Test Summary -
> > > -- The duration for case-2 was reduced to 2-3 minutes, matching the
> > > times of the other cases.
> > > -- The test revealed that the overhead in case-2 was not due to
> > > commit_ts fetching (GetTupleCommitTs).
> > > -- The additional action in case-2 was the error logging of all 10
> > > million update_differ conflicts.
> > >
> >
> > According to me, this last point is key among all tests which will
> > decide whether we should have a new subscription option like
> > detect_conflict or not. I feel this is the worst case where all the
> > row updates have conflicts and the majority of time is spent writing
> > LOG messages. Now, for this specific case, if one wouldn't have
> > enabled track_commit_timestamp then there would be no difference as
> > seen in case-4. So, I don't see this as a reason to introduce a new
> > subscription option like detect_conflicts, if one wants to avoid such
> > an overhead, she shouldn't have enabled track_commit_timestamp in the
> > first place to detect conflicts. Also, even without this, we would see
> > similar overhead in the case of update/delete_missing where we LOG
> > when the tuple to modify is not found.
> >
>
> Overall, it looks okay to get rid of the 'detect_conflict' parameter.
> My only concern here is the purpose/use-cases of
> 'track_commit_timestamp'.  Is the only purpose of enabling
> 'track_commit_timestamp' is to detect conflicts? I couldn't find much
> in the doc on this. Can there be a case where a user wants to enable
> 'track_commit_timestamp' for any other purpose without enabling
> subscription's conflict detection?
>

I am not aware of any other use case for 'track_commit_timestamp' GUC.
As per my understanding, commit timestamp is primarily required for
conflict detection and resolution. We can probably add a description
in 'track_commit_timestamp' GUC about its usage along with this patch.

-- 
With Regards,
Amit Kapila.