On Mon, Aug 5, 2024 at 10:05 AM shveta malik <shveta.ma...@gmail.com> wrote: > > On Mon, Aug 5, 2024 at 9:19 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > > > On Fri, Aug 2, 2024 at 6:28 PM Nisha Moond <nisha.moond...@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.
