Re: Conflict Detection and Resolution

On Thu, Aug 29, 2024 at 4:43 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Aug 23, 2024 at 10:39 AM shveta malik <shveta(dot)malik(at)gmail(dot)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