On Thu, Aug 29, 2024 at 4:43 PM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Fri, Aug 23, 2024 at 10:39 AM shveta malik <shveta.ma...@gmail.com> wrote: > > > > Please find issues which need some thoughts and approval for > > time-based resolution and clock-skew. > > > > 1) > > Time based conflict resolution and two phase transactions: > > > > Time based conflict resolution (last_update_wins) is the one > > resolution which will not result in data-divergence considering > > clock-skew is taken care of. But when it comes to two-phase > > transactions, it might not be the case. For two-phase transaction, we > > do not have commit timestamp when the changes are being applied. Thus > > for time-based comparison, initially it was decided to user prepare > > timestamp but it may result in data-divergence. Please see the > > example at [1]. > > > > Example at [1] is a tricky situation, and thus in the initial draft, > > we decided to restrict usage of 2pc and CDR together. The plan is: > > > > a) During Create subscription, if the user has given last_update_wins > > resolver for any conflict_type and 'two_phase' is also enabled, we > > ERROR out. > > b) During Alter subscription, if the user tries to update resolver to > > 'last_update_wins' but 'two_phase' is enabled, we error out. > > > > Another solution could be to save both prepare_ts and commit_ts. And > > when any txn comes for conflict resolution, we first check if > > prepare_ts is available, use that else use commit_ts. Availability of > > prepare_ts would indicate it was a prepared txn and thus even if it is > > committed, we should use prepare_ts for comparison for consistency. > > This will have some overhead of storing prepare_ts along with > > commit_ts. But if the number of prepared txns are reasonably small, > > this overhead should be less. > > > > Yet another idea is that if the conflict is detected and the > resolution strategy is last_update_wins then from that point we start > writing all the changes to the file similar to what we do for > streaming mode and only once commit_prepared arrives, we will read and > apply changes. That will solve this problem. > > > We currently plan to go with restricting 2pc and last_update_wins > > together, unless others have different opinions. > > > > Sounds reasonable but we should add comments on the possible solution > like the one I have mentioned so that we can extend it afterwards. >
Done, v12-004 patch has the comments for the possible solution. > > ~~ > > > > 2) > > parallel apply worker and conflict-resolution: > > As discussed in [2] (see last paragraph in [2]), for streaming of > > in-progress transactions by parallel worker, we do not have > > commit-timestamp with each change and thus it makes sense to disable > > parallel apply worker with CDR. The plan is to not start parallel > > apply worker if 'last_update_wins' is configured for any > > conflict_type. > > > > The other idea is that we can let the changes written to file if any > conflict is detected and then at commit time let the remaining changes > be applied by apply worker. This can introduce some complexity, so > similar to two_pc we can extend this functionality later. > v12-004 patch has the comments to extend it later. > > ~~ > > > > 3) > > parallel apply worker and clock skew management: > > Regarding clock-skew management as discussed in [3], we will wait for > > the local clock to come within tolerable range during 'begin' rather > > than before 'commit'. And this wait needs commit-timestamp in the > > beginning, thus we plan to restrict starting pa-worker even when > > clock-skew related GUCs are configured. > > > > Earlier we had restricted both 2pc and parallel worker worker start > > when detect_conflict was enabled, but now since detect_conflict > > parameter is removed, we will change the implementation to restrict > > all 3 above cases when last_update_wins is configured. When the > > changes are done, we will post the patch. > > > > At this stage, we are not sure how we want to deal with clock skew. > There is an argument that clock-skew should be handled outside the > database, so we can probably have the clock-skew-related stuff in a > separate patch. > Separated the clock-skew related code in v12-005 patch. -- Thanks, Nisha
