Re: Conflict detection and logging in logical replication

From: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
To: "Zhijie Hou (Fujitsu)" <houzj(dot)fnst(at)fujitsu(dot)com>
Cc: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, shveta malik <shveta(dot)malik(at)gmail(dot)com>, Nisha Moond <nisha(dot)moond412(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Jan Wieck <jan(at)wi3ck(dot)info>, Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Subject: Re: Conflict detection and logging in logical replication
Date: 2024-07-30 08:41:40
Message-ID: CAFiTN-tYdN63U=d8V8rBfRtFmhZ=QQX7jEmj1cdWMe_NM+7=TQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Jul 30, 2024 at 1:49 PM Zhijie Hou (Fujitsu)
<houzj(dot)fnst(at)fujitsu(dot)com> wrote:
>
> > On Monday, July 29, 2024 6:59 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com>
> > wrote:
> > >
> > > On Mon, Jul 29, 2024 at 11:44 AM Zhijie Hou (Fujitsu)
> > > <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > > >
> > >
> > > I was going through v7-0001, and I have some initial comments.
> >
> > Thanks for the comments !
> >
> > >
> > > @@ -536,11 +542,9 @@ ExecCheckIndexConstraints(ResultRelInfo
> > > *resultRelInfo, TupleTableSlot *slot,
> > > ExprContext *econtext;
> > > Datum values[INDEX_MAX_KEYS];
> > > bool isnull[INDEX_MAX_KEYS];
> > > - ItemPointerData invalidItemPtr;
> > > bool checkedIndex = false;
> > >
> > > ItemPointerSetInvalid(conflictTid);
> > > - ItemPointerSetInvalid(&invalidItemPtr);
> > >
> > > /*
> > > * Get information from the result relation info structure.
> > > @@ -629,7 +633,7 @@ ExecCheckIndexConstraints(ResultRelInfo
> > > *resultRelInfo, TupleTableSlot *slot,
> > >
> > > satisfiesConstraint =
> > > check_exclusion_or_unique_constraint(heapRelation, indexRelation,
> > > - indexInfo, &invalidItemPtr,
> > > + indexInfo, &slot->tts_tid,
> > > values, isnull, estate, false,
> > > CEOUC_WAIT, true,
> > > conflictTid);
> > >
> > > What is the purpose of this change? I mean
> > > 'check_exclusion_or_unique_constraint' says that 'tupleid'
> > > should be invalidItemPtr if the tuple is not yet inserted and
> > > ExecCheckIndexConstraints is called by ExecInsert before inserting the
> > tuple.
> > > So what is this change?
> >
> > Because the function ExecCheckIndexConstraints() is now invoked after
> > inserting a tuple (in the patch). So, we need to ignore the newly inserted tuple
> > when checking conflict in check_exclusion_or_unique_constraint().
> >
> > > Would this change ExecInsert's behavior as well?
> >
> > Thanks for pointing it out, I will check and reply.
>
> After checking, I think it may affect ExecInsert's behavior if the slot passed
> to ExecCheckIndexConstraints() comes from other tables (e.g. when executing
> INSERT INTO SELECT FROM othertbl), because the slot->tts_tid points to a valid
> position from another table in this case, which can cause the
> check_exclusion_or_unique_constraint to skip a tuple unexpectedly).
>
> I thought about two ideas to fix this: One is to reset the slot->tts_tid before
> calling ExecCheckIndexConstraints() in ExecInsert(), but I feel a bit
> uncomfortable to this since it is touching existing logic. So, another idea is to
> just add a new parameter 'tupletid' in ExecCheckIndexConstraints(), then pass
> tupletid=InvalidOffsetNumber in when invoke the function in ExecInsert() and
> pass a valid tupletid in the new code paths in the patch. The new
> 'tupletid' will be passed to check_exclusion_or_unique_constraint to
> skip the target tuple. I feel the second one maybe better.
>
> What do you think ?

Yes, the second approach seems good to me.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Ning 2024-07-30 08:47:35 psql client does not handle WSAEWOULDBLOCK on Windows
Previous Message Hayato Kuroda (Fujitsu) 2024-07-30 08:37:38 RE: speed up a logical replica setup