Re: Truncate in synchronous logical replication failed

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: "osumi(dot)takamichi(at)fujitsu(dot)com" <osumi(dot)takamichi(at)fujitsu(dot)com>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, "pgsql-hackers(at)lists(dot)postgresql(dot)org" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Truncate in synchronous logical replication failed
Date: 2021-04-22 09:33:22
Message-ID: CAA4eK1Jg2fB2phyWGxHV83qkE5pNuf82kU21Zzif6dpXDC+zbQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Apr 20, 2021 at 9:00 AM osumi(dot)takamichi(at)fujitsu(dot)com
<osumi(dot)takamichi(at)fujitsu(dot)com> wrote:
>
> On Tuesday, April 20, 2021 10:53 AM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> >
> > I reviewed the patch, ran make check, no issues. One minor comment:
> >
> > Could you add the comment similar to RelationGetIndexAttrBitmap() on why
> > the redo, it's not very obvious to someone reading the code, why we are
> > refetching the index list here.
> >
> > + /* Check if we need to redo */
> >
> > + newindexoidlist = RelationGetIndexList(relation);
> Yeah, makes sense. Fixed.

I don't think here we need to restart to get a stable list of indexes
as we do in RelationGetIndexAttrBitmap. The reason is here we build
the cache entry using a historic snapshot and all the later changes
are absorbed while decoding WAL. I have updated that and modified few
comments in the attached patch. Can you please test this in
clobber_cache_always mode? I think just testing
subscription/t/010_truncate.pl would be sufficient.

Now, this bug exists in prior versions (>= v11) where we have
introduced decoding of Truncate. Do we want to backpatch this? As no
user has reported this till now apart from Tang who I think got it
while doing some other patch testing, we might want to just push it in
HEAD. I am fine either way. Petr, others, do you have any opinion on
this matter?

--
With Regards,
Amit Kapila.

Attachment Content-Type Size
truncate_in_synchronous_logical_replication_v05.patch application/octet-stream 5.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2021-04-22 09:56:25 Re: ALTER TABLE .. DETACH PARTITION CONCURRENTLY
Previous Message Julien Rouhaud 2021-04-22 09:23:18 Re: Feature improvement: can we add queryId for pg_catalog.pg_stat_activity view?